[PATCH] D78341: Change callbr to only define its output SSA variable on the normal path, not the indirect targets.

[James Y Knight via Phabricator via llvm-commits]

jyknight updated this revision to Diff 258438.
jyknight added a comment.

Fix crash reported by Nick, InstructionSimplify was bypassing the
Dominators code in some cases, and doing its own thing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78341/new/

https://reviews.llvm.org/D78341

Files:
  llvm/docs/LangRef.rst
  llvm/lib/Analysis/InstructionSimplify.cpp
  llvm/lib/Analysis/ValueTracking.cpp
  llvm/lib/IR/Dominators.cpp


Index: llvm/lib/IR/Dominators.cpp
===================================================================
--- llvm/lib/IR/Dominators.cpp
+++ llvm/lib/IR/Dominators.cpp
@@ -134,7 +134,7 @@
   // 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)
@@ -168,6 +168,13 @@
     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);
 }
 
@@ -273,6 +280,13 @@
     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)
@@ -371,4 +385,3 @@
 void DominatorTreeWrapperPass::print(raw_ostream &OS, const Module *) const {
   DT.print(OS);
 }
-
Index: llvm/lib/Analysis/ValueTracking.cpp
===================================================================
--- llvm/lib/Analysis/ValueTracking.cpp
+++ llvm/lib/Analysis/ValueTracking.cpp
@@ -4642,7 +4642,7 @@
   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: {
Index: llvm/lib/Analysis/InstructionSimplify.cpp
===================================================================
--- llvm/lib/Analysis/InstructionSimplify.cpp
+++ llvm/lib/Analysis/InstructionSimplify.cpp
@@ -222,7 +222,7 @@
   // 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;
Index: llvm/docs/LangRef.rst
===================================================================
--- llvm/docs/LangRef.rst
+++ llvm/docs/LangRef.rst
@@ -7366,9 +7366,8 @@
 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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D78341.258438.patch
Type: text/x-patch
Size: 3248 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200417/c582819b/attachment.bin>