Skip to content

Commit

Permalink
[GlobalISel] Don't add duplicate successors to MBBs when translating …
Browse files Browse the repository at this point in the history
…indirectbr

This fixes a verifier failure on a bot:

http://green.lab.llvm.org/green/job/test-suite-verify-machineinstrs-aarch64-O0-g/

```
*** Bad machine code: MBB has duplicate entries in its successor list. ***
- function:    foo
- basic block: %bb.5 indirectgoto (0x7fe3d687ca08)
```

One of the GCC torture suite tests (pr70460.c) has an indirectbr instruction
which has duplicate blocks in its destination list.

According to the langref this is allowed:

> Blocks are allowed to occur multiple times in the destination list, though
> this isn’t particularly useful.
(https://www.llvm.org/docs/LangRef.html#indirectbr-instruction)

We don't allow this in MIR. So, when we translate such an instruction, the
verifier screams.

This patch makes `translateIndirectBr` check if a successor has already been
added to a block. If the successor is present, it is skipped rather than added
twice.

Differential Revision: https://reviews.llvm.org/D79609
  • Loading branch information
Jessica Paquette committed May 8, 2020
1 parent 0d2a0b4 commit f66309d
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 1 deletion.
9 changes: 8 additions & 1 deletion llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
Expand Up @@ -838,9 +838,16 @@ bool IRTranslator::translateIndirectBr(const User &U,
MIRBuilder.buildBrIndirect(Tgt);

// Link successors.
SmallPtrSet<const BasicBlock *, 32> AddedSuccessors;
MachineBasicBlock &CurBB = MIRBuilder.getMBB();
for (const BasicBlock *Succ : successors(&BrInst))
for (const BasicBlock *Succ : successors(&BrInst)) {
// It's legal for indirectbr instructions to have duplicate blocks in the
// destination list. We don't allow this in MIR. Skip anything that's
// already a successor.
if (!AddedSuccessors.insert(Succ).second)
continue;
CurBB.addSuccessor(&getMBB(*Succ));
}

return true;
}
Expand Down
@@ -0,0 +1,26 @@
; NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
; RUN: llc -O0 -mtriple=aarch64-apple-ios -global-isel -stop-after=irtranslator -verify-machineinstrs %s -o - | FileCheck %s
;
; Make sure that we don't duplicate successors/predecessors when translating
; indirectbr instructions with duplicate block labels.

define void @foo() {
; CHECK-LABEL: name: foo
; CHECK: bb.1 (%ir-block.0):
; CHECK: successors: %bb.2(0x2aaaaaaa), %bb.4(0x2aaaaaaa), %bb.3(0x2aaaaaaa)
; CHECK: [[DEF:%[0-9]+]]:_(p0) = G_IMPLICIT_DEF
; CHECK: G_BRINDIRECT [[DEF]](p0)
; CHECK: bb.2 (%ir-block.1):
; CHECK: successors:
; CHECK: bb.3 (%ir-block.2):
; CHECK: successors:
; CHECK: bb.4 (%ir-block.3):
; CHECK: RET_ReallyLR
indirectbr i8* undef, [label %1, label %3, label %2, label %3, label %3]
1:
unreachable
2:
unreachable
3:
ret void
}

0 comments on commit f66309d

Please sign in to comment.