[PATCH] D149592: [InstCombine] Fix bug in `FoldOpIntoSelect` where we would incorrectly fold `undef` as constant

Noah Goldstein via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 1 08:30:29 PDT 2023


goldstein.w.n created this revision.
goldstein.w.n added reviewers: nikic, majnemer, spatel.
Herald added subscribers: StephenFan, hiraditya, kristof.beyls.
Herald added a project: All.
goldstein.w.n requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

D146349 <https://reviews.llvm.org/D146349> Introduced the ability to use the information from the
`select` condition to deduce constants as we folded a binop into
select. I.e if the `select` cond was `icmp eq %A, 10`, then in the
true-arm of `select`, we would be able to replace usage of `A` with
`10`.

This is broken for vectors that contain `undef` elements. I.e with
`icmp eq %A, <10, undef>`, subsituting `<10, undef>` for `A` can
result in creating a more undefined result than we otherwise would
have.

We fix the issue with simply checking if the candidate constant for
substituting may contain `undef` elements and don't do it in that
case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149592

Files:
  llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
  llvm/test/Transforms/InstCombine/binop-select.ll


Index: llvm/test/Transforms/InstCombine/binop-select.ll
===================================================================
--- llvm/test/Transforms/InstCombine/binop-select.ll
+++ llvm/test/Transforms/InstCombine/binop-select.ll
@@ -6,6 +6,7 @@
 declare void @use_v2f16(<2 x half>)
 declare void @use_v2i8(<2 x i8>)
 declare i32 @llvm.sadd.sat.i32(i32, i32)
+declare <2 x i8> @llvm.sadd.sat.v2i8(<2 x i8>, <2 x i8>)
 
 define i32 @test1(i1 %c, i32 %x, i32 %y) {
 ; CHECK-LABEL: @test1(
@@ -109,6 +110,34 @@
   ret i32 %sub
 }
 
+define <2 x i8> @test_sub_dont_deduce_with_undef_cond_vec(<2 x i8> %x, <2 x i8> %y) {
+; CHECK-LABEL: @test_sub_dont_deduce_with_undef_cond_vec(
+; CHECK-NEXT:    [[C_NOT:%.*]] = icmp eq <2 x i8> [[X:%.*]], <i8 9, i8 undef>
+; CHECK-NEXT:    [[COND:%.*]] = select <2 x i1> [[C_NOT]], <2 x i8> <i8 7, i8 7>, <2 x i8> [[Y:%.*]]
+; CHECK-NEXT:    [[SUB:%.*]] = call <2 x i8> @llvm.sadd.sat.v2i8(<2 x i8> [[X]], <2 x i8> [[COND]])
+; CHECK-NEXT:    ret <2 x i8> [[SUB]]
+;
+  %c = icmp ne <2 x i8> %x, <i8 9, i8 undef>
+  %cond = select <2 x i1> %c, <2 x i8> %y, <2 x i8> <i8 7, i8 7>
+  %sub = call <2 x i8> @llvm.sadd.sat.v2i8(<2 x i8> %x, <2 x i8> %cond)
+  ret <2 x i8> %sub
+}
+
+
+define <2 x i8> @test_sub_deduce_with_undef_val_vec(<2 x i8> %x, <2 x i8> %y) {
+; CHECK-LABEL: @test_sub_deduce_with_undef_val_vec(
+; CHECK-NEXT:    [[C_NOT:%.*]] = icmp eq <2 x i8> [[X:%.*]], <i8 1, i8 2>
+; CHECK-NEXT:    [[TMP1:%.*]] = call <2 x i8> @llvm.sadd.sat.v2i8(<2 x i8> [[X]], <2 x i8> [[Y:%.*]])
+; CHECK-NEXT:    [[SUB:%.*]] = select <2 x i1> [[C_NOT]], <2 x i8> <i8 4, i8 -1>, <2 x i8> [[TMP1]]
+; CHECK-NEXT:    ret <2 x i8> [[SUB]]
+;
+  %c = icmp ne <2 x i8> %x, <i8 1, i8 2>
+  %cond = select <2 x i1> %c, <2 x i8> %y, <2 x i8> <i8 3, i8 undef>
+  %sub = call <2 x i8> @llvm.sadd.sat.v2i8(<2 x i8> %x, <2 x i8> %cond)
+  ret <2 x i8> %sub
+}
+
+
 define i32 @test6(i1 %c, i32 %x, i32 %y) {
 ; CHECK-LABEL: @test6(
 ; CHECK-NEXT:    [[COND:%.*]] = select i1 [[C:%.*]], i32 7, i32 [[X:%.*]]
Index: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
===================================================================
--- llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -1040,7 +1040,8 @@
                                        : SI->getFalseValue());
     } else if (match(SI->getCondition(),
                      m_ICmp(Pred, m_Specific(Op), m_Constant(C))) &&
-               Pred == (IsTrueArm ? ICmpInst::ICMP_EQ : ICmpInst::ICMP_NE)) {
+               Pred == (IsTrueArm ? ICmpInst::ICMP_EQ : ICmpInst::ICMP_NE) &&
+               !C->containsUndefOrPoisonElement()) {
       // Pass
     } else {
       C = dyn_cast<Constant>(Op);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D149592.518453.patch
Type: text/x-patch
Size: 2751 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230501/26f5b811/attachment.bin>


More information about the llvm-commits mailing list