Skip to content

Commit

Permalink
Change callbr to only define its output SSA variable on the normal
Browse files Browse the repository at this point in the history
path, not the indirect targets.

Fixes: PR45565.

Differential Revision: https://reviews.llvm.org/D78341
  • Loading branch information
jyknight committed Apr 23, 2020
1 parent 48e9ef4 commit 248a5db
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 7 deletions.
5 changes: 2 additions & 3 deletions llvm/docs/LangRef.rst
Expand Up @@ -7366,9 +7366,8 @@ instruction in most regards. The primary difference is that it
establishes an association with additional labels to define where control
flow goes after the call.

Outputs of a '``callbr``' instruction are valid only on the '``fallthrough``'
path. Use of outputs on the '``indirect``' path(s) results in :ref:`poison
values <poisonvalues>`.
The output values of a '``callbr``' instruction are available only to
the '``fallthrough``' block, not to any '``indirect``' blocks(s).

The only use of this today is to implement the "goto" feature of gcc inline
assembly where additional labels can be provided as locations for the inline
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Analysis/InstructionSimplify.cpp
Expand Up @@ -222,7 +222,7 @@ static bool valueDominatesPHI(Value *V, PHINode *P, const DominatorTree *DT) {
// Otherwise, if the instruction is in the entry block and is not an invoke,
// then it obviously dominates all phi nodes.
if (I->getParent() == &I->getFunction()->getEntryBlock() &&
!isa<InvokeInst>(I))
!isa<InvokeInst>(I) && !isa<CallBrInst>(I))
return true;

return false;
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Analysis/ValueTracking.cpp
Expand Up @@ -4641,7 +4641,7 @@ bool llvm::canCreatePoison(const Instruction *I) {
case Instruction::CallBr:
case Instruction::Invoke:
// Function calls can return a poison value even if args are non-poison
// values. CallBr returns poison when jumping to indirect labels.
// values.
return true;
case Instruction::InsertElement:
case Instruction::ExtractElement: {
Expand Down
17 changes: 15 additions & 2 deletions llvm/lib/IR/Dominators.cpp
Expand Up @@ -134,7 +134,7 @@ bool DominatorTree::dominates(const Instruction *Def,
// dominates every instruction in UseBB.
// A PHI is dominated only if the instruction dominates every possible use in
// the UseBB.
if (isa<InvokeInst>(Def) || isa<PHINode>(User))
if (isa<InvokeInst>(Def) || isa<CallBrInst>(Def) || isa<PHINode>(User))
return dominates(Def, UseBB);

if (DefBB != UseBB)
Expand Down Expand Up @@ -168,6 +168,13 @@ bool DominatorTree::dominates(const Instruction *Def,
return dominates(E, UseBB);
}

// Callbr results are similarly only usable in the default destination.
if (const auto *CBI = dyn_cast<CallBrInst>(Def)) {
BasicBlock *NormalDest = CBI->getDefaultDest();
BasicBlockEdge E(DefBB, NormalDest);
return dominates(E, UseBB);
}

return dominates(DefBB, UseBB);
}

Expand Down Expand Up @@ -273,6 +280,13 @@ bool DominatorTree::dominates(const Instruction *Def, const Use &U) const {
return dominates(E, U);
}

// Callbr results are similarly only usable in the default destination.
if (const auto *CBI = dyn_cast<CallBrInst>(Def)) {
BasicBlock *NormalDest = CBI->getDefaultDest();
BasicBlockEdge E(DefBB, NormalDest);
return dominates(E, U);
}

// If the def and use are in different blocks, do a simple CFG dominator
// tree query.
if (DefBB != UseBB)
Expand Down Expand Up @@ -371,4 +385,3 @@ void DominatorTreeWrapperPass::verifyAnalysis() const {
void DominatorTreeWrapperPass::print(raw_ostream &OS, const Module *) const {
DT.print(OS);
}

26 changes: 26 additions & 0 deletions llvm/test/CodeGen/X86/callbr-codegenprepare.ll
@@ -0,0 +1,26 @@
;; RUN: opt -S -codegenprepare < %s | FileCheck %s

;; Ensure that codegenprepare (via InstSimplify) doesn't eliminate the
;; phi here (which would cause a module verification error).

;; CHECK: phi

target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

declare void @foo(i32)

define dso_local i32 @futex_lock_pi_atomic() local_unnamed_addr {
entry:
%0 = callbr i32 asm "", "=r,X,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@futex_lock_pi_atomic, %b.exit))
to label %asm.fallthrough.i [label %b.exit]

asm.fallthrough.i:
br label %b.exit

b.exit:
%g.0 = phi i32 [ %0, %asm.fallthrough.i ], [ undef, %entry ]
tail call void @foo(i32 %g.0)
ret i32 undef
}

13 changes: 13 additions & 0 deletions llvm/test/Verifier/dominates.ll
Expand Up @@ -68,3 +68,16 @@ next:
; CHECK-NEXT: %y = phi i32 [ 0, %entry ]
; CHECK-NEXT: %x = phi i32 [ %y, %entry ]
}

define i32 @f6(i32 %x) {
bb0:
%y1 = callbr i32 asm "", "=r"() to label %bb1 [label %bb2]
bb1:
ret i32 0
bb2:
ret i32 %y1
; CHECK: Instruction does not dominate all uses!
; CHECK-NEXT: %y1 = callbr i32 asm "", "=r"()
; CHECK-NEXT: to label %bb1 [label %bb2]
; CHECK-NEXT: ret i32 %y1
}

0 comments on commit 248a5db

Please sign in to comment.