[InstCombine] Fix constexpr issue in select combining

[RKSimon]

Summary

As discussed by Andrea on PR30486, we have an unsafe cast to an Instruction type in the select combine which doesn't take into account that it could be a ConstantExpr instead.

[andreadb]

Hi Simon,

the patch looks good to me.
I suggest to improve the test case by adding multiple cases to the switch statement. That would prove that the optimization still works okay even with constant expressions (see below).

Example:

@g = global i32 0

define i32 @func() {
  switch i32 add (i32 ptrtoint (i32* @g to i32), i32 -1), label %x [
    i32 1, label %one
    i32 2, label %two
  ]
x:
  ret i32 0

one:
  ret i32 1

two:
  ret i32 2
}

The above case should be combined to this:

define i32 @func() {
  switch i32 ptrtoint (i32* @g to i32), label %x [
    i32 2, label %one
    i32 3, label %two
  ]

x:                                                ; preds = %0
  ret i32 0

one:                                              ; preds = %0
  ret i32 1

two:                                              ; preds = %0
  ret i32 2

Thanks!
Andrea

[RKSimon]

Fixed in rL284000 - I added Andrea's alternate test case so we test both single case and multiple case switches.

lib/Transforms/InstCombine/InstructionCombining.cpp

if (NewWidth > 0 && NewWidth < BitWidth) {
     Value *NewCond = Builder->CreateTrunc(Cond, Ty, "trunc");
     SI.setCondition(NewCond);
 
     for (auto &C : SI.cases())
       static_cast<SwitchInst::CaseIt *>(&C)->setValue(ConstantInt::get(
           SI.getContext(), C.getCaseValue()->getValue().trunc(NewWidth)));
   }
 
+  Value *Op0 = nullptr;
   ConstantInt *AddRHS = nullptr;
-  if (match(Cond, m_Add(m_Value(), m_ConstantInt(AddRHS)))) {
-    Instruction *I = cast<Instruction>(Cond);
+  if (match(Cond, m_Add(m_Value(Op0), m_ConstantInt(AddRHS)))) {
     // Change 'switch (X+4) case 1:' into 'switch (X) case -3'.
     for (SwitchInst::CaseIt i = SI.case_begin(), e = SI.case_end(); i != e;
          ++i) {
       ConstantInt *CaseVal = i.getCaseValue();
       Constant *LHS = CaseVal;
       if (TruncCond) {
         LHS = LeadingKnownZeros
                   ? ConstantExpr::getZExt(CaseVal, Cond->getType())
                   : ConstantExpr::getSExt(CaseVal, Cond->getType());
       }
       Constant *NewCaseVal = ConstantExpr::getSub(LHS, AddRHS);
       assert(isa<ConstantInt>(NewCaseVal) &&
              "Result of expression should be constant");
       i.setValue(cast<ConstantInt>(NewCaseVal));
     }
-    SI.setCondition(I->getOperand(0));
-    Worklist.Add(I);
+    SI.setCondition(Op0);
+    if (auto *CondI = dyn_cast<Instruction>(Cond))
+      Worklist.Add(CondI);
     return &SI;
   }
 
   return TruncCond ? &SI : nullptr;
 }
 
 Instruction *InstCombiner::visitExtractValueInst(ExtractValueInst &EV) {
   Value *Agg = EV.getAggregateOperand();

test/Transforms/InstCombine/switch-constant-expr.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -instcombine -S | FileCheck %s
+
+@g = global i32 0
+
+; PR30486
+define i32 @func() {
+; CHECK-LABEL: @func(
+; CHECK-NEXT:    switch i32 ptrtoint (i32* @g to i32), label %x [
+; CHECK-NEXT:    ]
+; CHECK:       x:
+; CHECK-NEXT:    ret i32 0
+;
+  switch i32 add (i32 ptrtoint (i32* @g to i32), i32 -1), label %x []
+x:
+  ret i32 0
+}