[llvm] 6cdc229 - [InstCombine] Fix bug in `FoldOpIntoSelect` where we would incorrectly fold `undef` as constant

Noah Goldstein via llvm-commits llvm-commits at lists.llvm.org
Mon May 1 15:24:06 PDT 2023


Author: Noah Goldstein
Date: 2023-05-01T17:23:54-05:00
New Revision: 6cdc229a64be8dacba40d30a9032c14f51ee30c0

URL: https://github.com/llvm/llvm-project/commit/6cdc229a64be8dacba40d30a9032c14f51ee30c0
DIFF: https://github.com/llvm/llvm-project/commit/6cdc229a64be8dacba40d30a9032c14f51ee30c0.diff

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

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.

Reviewed By: nikic

Differential Revision: https://reviews.llvm.org/D149592

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 82e35d3c6fed..f0a7a52d9121 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -1040,7 +1040,8 @@ static Constant *constantFoldOperationIntoSelectOperand(Instruction &I,
                                        : 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) &&
+               isGuaranteedNotToBeUndefOrPoison(C)) {
       // Pass
     } else {
       C = dyn_cast<Constant>(Op);

diff  --git a/llvm/test/Transforms/InstCombine/binop-select.ll b/llvm/test/Transforms/InstCombine/binop-select.ll
index da6c17984eb9..a59e19897f06 100644
--- a/llvm/test/Transforms/InstCombine/binop-select.ll
+++ b/llvm/test/Transforms/InstCombine/binop-select.ll
@@ -6,6 +6,7 @@ declare void @use_f32(float)
 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,47 @@ define i32 @test_sub_deduce_false(i32 %x, i32 %y) {
   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_dont_deduce_with_poison_cond_vec(<2 x i8> %x, <2 x i8> %y) {
+; CHECK-LABEL: @test_sub_dont_deduce_with_poison_cond_vec(
+; CHECK-NEXT:    [[C_NOT:%.*]] = icmp eq <2 x i8> [[X:%.*]], <i8 poison, i8 9>
+; 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 poison, i8 9>
+  %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:%.*]]


        


More information about the llvm-commits mailing list