[llvm] 33590ed - [InstCombine] fix another poison-unsafe select transformation

[Juneyoung Lee via llvm-commits]

Author: Juneyoung Lee
Date: 2021-03-08T02:11:04+09:00
New Revision: 33590ed4f264557ff34702dd61d50484962d667b

URL: https://github.com/llvm/llvm-project/commit/33590ed4f264557ff34702dd61d50484962d667b
DIFF: https://github.com/llvm/llvm-project/commit/33590ed4f264557ff34702dd61d50484962d667b.diff

LOG: [InstCombine] fix another poison-unsafe select transformation

This fixes another unsafe select folding by disabling it if
EnableUnsafeSelectTransform is set to false.

EnableUnsafeSelectTransform's default value is true, hence it won't
affect generated code (unless the flag is explicitly set to false).

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
    llvm/test/Transforms/InstCombine/select-safe-bool-transforms.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
index f26c194d31b9..7d9383c346c8 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -2890,7 +2890,11 @@ Instruction *InstCombinerImpl::visitSelectInst(SelectInst &SI) {
       // shortening paths for the values (this helps getUnderlyingObjects() for
       // example).
       if (TrueSI->getFalseValue() == FalseVal && TrueSI->hasOneUse()) {
-        Value *And = Builder.CreateAnd(CondVal, TrueSI->getCondition());
+        // Simply merging into and i1 isn't poison-safe. Do it only when
+        // EnableUnsafeSelectTransform is set.
+        Value *And = EnableUnsafeSelectTransform ?
+            Builder.CreateAnd(CondVal, TrueSI->getCondition()) :
+            Builder.CreateLogicalAnd(CondVal, TrueSI->getCondition());
         replaceOperand(SI, 0, And);
         replaceOperand(SI, 1, TrueSI->getTrueValue());
         return &SI;
@@ -2907,7 +2911,11 @@ Instruction *InstCombinerImpl::visitSelectInst(SelectInst &SI) {
       }
       // select(C0, a, select(C1, a, b)) -> select(C0|C1, a, b)
       if (FalseSI->getTrueValue() == TrueVal && FalseSI->hasOneUse()) {
-        Value *Or = Builder.CreateOr(CondVal, FalseSI->getCondition());
+        // Simply merging into or i1 isn't poison-safe. Do it only when
+        // EnableUnsafeSelectTransform is set.
+        Value *Or = EnableUnsafeSelectTransform ?
+            Builder.CreateOr(CondVal, FalseSI->getCondition()) :
+            Builder.CreateLogicalOr(CondVal, FalseSI->getCondition());
         replaceOperand(SI, 0, Or);
         replaceOperand(SI, 2, FalseSI->getFalseValue());
         return &SI;

diff  --git a/llvm/test/Transforms/InstCombine/select-safe-bool-transforms.ll b/llvm/test/Transforms/InstCombine/select-safe-bool-transforms.ll
index fd1e1be65db7..83275ec21c3c 100644
--- a/llvm/test/Transforms/InstCombine/select-safe-bool-transforms.ll
+++ b/llvm/test/Transforms/InstCombine/select-safe-bool-transforms.ll
@@ -262,7 +262,7 @@ define i1 @land_land_right1(i1 %A, i1 %B) {
 }
 define i1 @land_land_right2(i1 %A, i1 %B) {
 ; CHECK-LABEL: @land_land_right2(
-; CHECK-NEXT:    [[TMP1:%.*]] = and i1 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    [[TMP1:%.*]] = select i1 [[A:%.*]], i1 [[B:%.*]], i1 false
 ; CHECK-NEXT:    ret i1 [[TMP1]]
 ;
   %c = select i1 %B, i1 %A, i1 false
@@ -426,7 +426,7 @@ define i1 @lor_lor_right1(i1 %A, i1 %B) {
 }
 define i1 @lor_lor_right2(i1 %A, i1 %B) {
 ; CHECK-LABEL: @lor_lor_right2(
-; CHECK-NEXT:    [[TMP1:%.*]] = or i1 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    [[TMP1:%.*]] = select i1 [[A:%.*]], i1 true, i1 [[B:%.*]]
 ; CHECK-NEXT:    ret i1 [[TMP1]]
 ;
   %c = select i1 %B, i1 true, i1 %A