[SimplifyCFG] Don't create PHI nodes for constant bundle operands

[sanjoy]

Summary

Constant bundle operands may need to retain their constant-ness for
correctness. I'll admit that this is slightly odd, but it looks like
SimplifyCFG already does this for things like @llvm.frameaddress and
@llvm.stackmap, so I suppose adding one more case is not a big deal.

It is possible to add a mechanism to denote bundle operands that need to
remain constants, but that's probably too complicated for the time
being.

[jmolloy]

LGTM, with the extra comment added.

lib/Transforms/Utils/SimplifyCFG.cpp

1368

Could you please add a comment here as to why - something like what you wrote in your phab commit message. Just so we know that it's deliberate and not a workaround for something else.

include/llvm/IR/CallSite.h

#define CALLSITE_DELEGATE_SETTER(METHOD) \
   Optional<OperandBundleUse> getOperandBundle(uint32_t ID) const {
     CALLSITE_DELEGATE_GETTER(getOperandBundle(ID));
   }
 
   unsigned countOperandBundlesOfType(uint32_t ID) const {
     CALLSITE_DELEGATE_GETTER(countOperandBundlesOfType(ID));
   }
 
+  bool isBundleOperand(unsigned Idx) const {
+    CALLSITE_DELEGATE_GETTER(isBundleOperand(Idx));
+  }
+
   IterTy arg_begin() const {
     CALLSITE_DELEGATE_GETTER(arg_begin());
   }
 
   IterTy arg_end() const {
     CALLSITE_DELEGATE_GETTER(arg_end());
   }
 

include/llvm/IR/InstrTypes.h

public:
   }
 
   /// \brief Return the index of the last bundle operand in the Use array.
   unsigned getBundleOperandsEndIndex() const {
     assert(hasOperandBundles() && "Don't call otherwise!");
     return bundle_op_info_end()[-1].End;
   }
 
+  /// Return true if the operand at index \p Idx is a bundle operand.
+  bool isBundleOperand(unsigned Idx) const {
+    return hasOperandBundles() && Idx >= getBundleOperandsStartIndex() &&
+           Idx < getBundleOperandsEndIndex();
+  }
+
   /// \brief Return the total number operands (not operand bundles) used by
   /// every operand bundle in this OperandBundleUser.
   unsigned getNumTotalBundleOperands() const {
     if (!hasOperandBundles())
       return 0;
 
     unsigned Begin = getBundleOperandsStartIndex();
     unsigned End = getBundleOperandsEndIndex();

lib/Transforms/Utils/SimplifyCFG.cpp

static bool canReplaceOperandWithVariable(const Instruction *I,
   switch (I->getOpcode()) {
   default:
     return true;
   case Instruction::Call:
   case Instruction::Invoke:
     // FIXME: many arithmetic intrinsics have no issue taking a
     // variable, however it's hard to distingish these from
     // specials such as @llvm.frameaddress that require a constant.
-    return !isa<IntrinsicInst>(I);
+    if (isa<IntrinsicInst>(I))
+      return false;
+
+    if (ImmutableCallSite(I).isBundleOperand(OpIdx))
+      return false;
+
+    return true;
+
   case Instruction::ShuffleVector:
     // Shufflevector masks are constant.
     return OpIdx != 2;
   case Instruction::ExtractValue:
   case Instruction::InsertValue:
     // All operands apart from the first are constant.
     return OpIdx == 0;
   case Instruction::Alloca:

test/Transforms/SimplifyCFG/sink-common-code.ll

if.end:
   ret i32 %ret
 }
 
 ; CHECK-LABEL: test_pr30373b
 ; CHECK: lshr
 ; CHECK-NOT: exact
 ; CHECK: }
 
+declare i32 @call_target()
+
+define void @test_operand_bundles(i1 %cond, i32* %ptr) {
+entry:
+  br i1 %cond, label %left, label %right
+
+left:
+  %val0 = call i32 @call_target() [ "deopt"(i32 10) ]
+  store i32 %val0, i32* %ptr
+  br label %merge
+
+right:
+  %val1 = call i32 @call_target() [ "deopt"(i32 20) ]
+  store i32 %val1, i32* %ptr
+  br label %merge
+
+merge:
+  ret void
+}
+
+; CHECK-LABEL: @test_operand_bundles(
+; CHECK: left:
+; CHECK-NEXT:   %val0 = call i32 @call_target() [ "deopt"(i32 10) ]
+; CHECK: right:
+; CHECK-NEXT:   %val1 = call i32 @call_target() [ "deopt"(i32 20) ]
+
 ; CHECK: !0 = !{!1, !1, i64 0}
 ; CHECK: !1 = !{!"float", !2}
 ; CHECK: !2 = !{!"an example type tree"}