[InstCombine] Preserve metadata when combining select+binop

[chfast]

Summary

Fixes https://github.com/llvm/llvm-project/issues/63910

[chfast]

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

541

I believe there are more case in the file where metadata copy is missed. I will try to identify them and create more tests cases. Can be in this review on later, depending on reviewers' suggestion.

llvm/test/Transforms/InstCombine/unpredictable-select.ll

1 ↗

(On Diff #541012)

I created this test file. Is this good location?

[goldstein.w.n]

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

541

Are there any other binops you want to preserve this flag for?

llvm/test/Transforms/InstCombine/unpredictable-select.ll

1 ↗

(On Diff #541012)

Its fine.

[goldstein.w.n]

Can you make the summary a bit more explinatory than just "fixed..."

[nikic]

llvm/test/Transforms/InstCombine/unpredictable-select.ll

1 ↗

(On Diff #541012)

I would add this to the select_meta.ll test.

[chfast]

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

541

I think it make sense for any binop because the condition of the select in this transformation remains unchanged (i.e. !unpredictable is still applicable).

[goldstein.w.n]

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

541

I think you have this in the right place for most binops, but you should probably add additional tests for things like:
sub, or, xor, and, mul, div, rem, llvm.min, llvm.max, etc...

you probably don't need all but some variety of those (like 1/2 bitwise, div or rem, a few intrinsics) should provide better coverage.

llvm/test/Transforms/InstCombine/unpredictable-select.ll

1 ↗

(On Diff #541012)

+1

[chfast]

Rebased on new tests cases.

[chfast]

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

541

I added tests for most instructions from getSelectFoldableOperands. There are no any intrinsics there.

[nikic]

LGTM

llvm/test/Transforms/InstCombine/select_meta.ll

317–318

Drop the TODOs.

[chfast]

Removed TODO comments from tests.

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

if (!C1I.isZero() && !C2I.isZero()) // One side must be zero.
     return false;
   return C1I.isOne() || C1I.isAllOnes() || C2I.isOne() || C2I.isAllOnes();
 }
 
 /// Try to fold the select into one of the operands to allow further
 /// optimization.
 Instruction *InstCombinerImpl::foldSelectIntoOp(SelectInst &SI, Value *TrueVal,
                                                 Value *FalseVal) {
-  // See the comment above GetSelectFoldableOperands for a description of the
+  // See the comment above getSelectFoldableOperands for a description of the
   // transformation we are doing here.
   auto TryFoldSelectIntoOp = [&](SelectInst &SI, Value *TrueVal,
                                  Value *FalseVal,
                                  bool Swapped) -> Instruction * {
     auto *TVI = dyn_cast<BinaryOperator>(TrueVal);
     if (!TVI || !TVI->hasOneUse() || isa<Constant>(FalseVal))
       return nullptr;
 
auto TryFoldSelectIntoOp = [&](SelectInst &SI, Value *TrueVal,
     Value *OOp = TVI->getOperand(2 - OpToFold);
     // Avoid creating select between 2 constants unless it's selecting
     // between 0, 1 and -1.
     const APInt *OOpC;
     bool OOpIsAPInt = match(OOp, m_APInt(OOpC));
     if (!isa<Constant>(OOp) ||
         (OOpIsAPInt && isSelect01(C->getUniqueInteger(), *OOpC))) {
       Value *NewSel = Builder.CreateSelect(SI.getCondition(), Swapped ? C : OOp,
-                                           Swapped ? OOp : C);
+                                           Swapped ? OOp : C, "", &SI);
       if (isa<FPMathOperator>(&SI))
         cast<Instruction>(NewSel)->setFastMathFlags(FMF);
       NewSel->takeName(TVI);
       BinaryOperator *BO =
           BinaryOperator::Create(TVI->getOpcode(), FalseVal, NewSel);
       BO->copyIRFlags(TVI);
       return BO;
     }

llvm/test/Transforms/InstCombine/select_meta.ll

 ; CHECK-LABEL: @not_cond_vec_undef(
 ; CHECK-NEXT:    [[R:%.*]] = select <2 x i1> [[C:%.*]], <2 x i32> [[FV:%.*]], <2 x i32> [[TV:%.*]], !prof [[PROF1]]
 ; CHECK-NEXT:    ret <2 x i32> [[R]]
 ;
   %notc = xor <2 x i1> %c, <i1 undef, i1 true>
   %r = select <2 x i1> %notc, <2 x i32> %tv, <2 x i32> %fv, !prof !1
   ret <2 x i32> %r
 }
 
-; TODO: The metadata should be preserved.
 define i64 @select_add(i1 %cond, i64 %x, i64 %y) {
 ; CHECK-LABEL: @select_add(
-; CHECK-NEXT:    [[OP:%.*]] = select i1 [[COND:%.*]], i64 [[Y:%.*]], i64 0
+; CHECK-NEXT:    [[OP:%.*]] = select i1 [[COND:%.*]], i64 [[Y:%.*]], i64 0, !prof [[PROF0]], !unpredictable !2
 ; CHECK-NEXT:    [[RET:%.*]] = add i64 [[OP]], [[X:%.*]]
 ; CHECK-NEXT:    ret i64 [[RET]]
 ;
   %op = add i64 %x, %y
   %ret = select i1 %cond, i64 %op, i64 %x, !prof !1, !unpredictable !3
   ret i64 %ret
 }
 
-; TODO: The metadata should be preserved.
 define <2 x i32> @select_or(<2 x i1> %cond, <2 x i32> %x, <2 x i32> %y) {
 ; CHECK-LABEL: @select_or(
-; CHECK-NEXT:    [[OP:%.*]] = select <2 x i1> [[COND:%.*]], <2 x i32> [[Y:%.*]], <2 x i32> zeroinitializer
+; CHECK-NEXT:    [[OP:%.*]] = select <2 x i1> [[COND:%.*]], <2 x i32> [[Y:%.*]], <2 x i32> zeroinitializer, !prof [[PROF0]], !unpredictable !2
 ; CHECK-NEXT:    [[RET:%.*]] = or <2 x i32> [[OP]], [[X:%.*]]
 ; CHECK-NEXT:    ret <2 x i32> [[RET]]
 ;
   %op = or <2 x i32> %x, %y
   %ret = select <2 x i1> %cond, <2 x i32> %op, <2 x i32> %x, !prof !1, !unpredictable !3
   ret <2 x i32> %ret
 }
 
-; TODO: The metadata should be preserved.
 define i17 @select_sub(i1 %cond, i17 %x, i17 %y) {
 ; CHECK-LABEL: @select_sub(
-; CHECK-NEXT:    [[OP:%.*]] = select i1 [[COND:%.*]], i17 [[Y:%.*]], i17 0
+; CHECK-NEXT:    [[OP:%.*]] = select i1 [[COND:%.*]], i17 [[Y:%.*]], i17 0, !prof [[PROF0]], !unpredictable !2
 ; CHECK-NEXT:    [[RET:%.*]] = sub i17 [[X:%.*]], [[OP]]
 ; CHECK-NEXT:    ret i17 [[RET]]
 ;
   %op = sub i17 %x, %y
   %ret = select i1 %cond, i17 %op, i17 %x, !prof !1, !unpredictable !3
   ret i17 %ret
 }
 
-; TODO: The metadata should be preserved.
 define i128 @select_ashr(i1 %cond, i128 %x, i128 %y) {
 ; CHECK-LABEL: @select_ashr(
-; CHECK-NEXT:    [[OP:%.*]] = select i1 [[COND:%.*]], i128 [[Y:%.*]], i128 0
+; CHECK-NEXT:    [[OP:%.*]] = select i1 [[COND:%.*]], i128 [[Y:%.*]], i128 0, !prof [[PROF0]], !unpredictable !2
 ; CHECK-NEXT:    [[RET:%.*]] = ashr i128 [[X:%.*]], [[OP]]
 ; CHECK-NEXT:    ret i128 [[RET]]
 ;
   %op = ashr i128 %x, %y
   %ret = select i1 %cond, i128 %op, i128 %x, !prof !1, !unpredictable !3
   ret i128 %ret
 }
 
-; TODO: The metadata should be preserved.
 define double @select_fmul(i1 %cond, double %x, double %y) {
 ; CHECK-LABEL: @select_fmul(
-; CHECK-NEXT:    [[OP:%.*]] = select i1 [[COND:%.*]], double [[Y:%.*]], double 1.000000e+00
+; CHECK-NEXT:    [[OP:%.*]] = select i1 [[COND:%.*]], double [[Y:%.*]], double 1.000000e+00, !prof [[PROF0]], !unpredictable !2
 ; CHECK-NEXT:    [[RET:%.*]] = fmul double [[OP]], [[X:%.*]]
 ; CHECK-NEXT:    ret double [[RET]]
 ;
   %op = fmul double %x, %y
   %ret = select i1 %cond, double %op, double %x, !prof !1, !unpredictable !3
   ret double %ret
 }
 
-; TODO: The metadata should be preserved.
 define <2 x float> @select_fdiv(i1 %cond, <2 x float> %x, <2 x float> %y) {
 ; CHECK-LABEL: @select_fdiv(
-; CHECK-NEXT:    [[OP:%.*]] = select i1 [[COND:%.*]], <2 x float> [[Y:%.*]], <2 x float> <float 1.000000e+00, float 1.000000e+00>
+; CHECK-NEXT:    [[OP:%.*]] = select i1 [[COND:%.*]], <2 x float> [[Y:%.*]], <2 x float> <float 1.000000e+00, float 1.000000e+00>, !prof [[PROF0]], !unpredictable !2
 ; CHECK-NEXT:    [[RET:%.*]] = fdiv <2 x float> [[X:%.*]], [[OP]]
 ; CHECK-NEXT:    ret <2 x float> [[RET]]
 ;
   %op = fdiv <2 x float> %x, %y
   %ret = select i1 %cond, <2 x float> %op, <2 x float> %x, !prof !1, !unpredictable !3
   ret <2 x float> %ret
 }
 
 !1 = !{!"branch_weights", i32 2, i32 10}
 !2 = !{!"branch_weights", i32 3, i32 10}
 !3 = !{}
 
 ;.
 ; CHECK: attributes #[[ATTR0:[0-9]+]] = { nounwind }
 ; CHECK: attributes #[[ATTR1:[0-9]+]] = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }
 ;.
 ; CHECK: [[PROF0]] = !{!"branch_weights", i32 2, i32 10}
 ; CHECK: [[PROF1]] = !{!"branch_weights", i32 10, i32 2}
+; CHECK: [[META2:![0-9]+]] = !{}
 ;.