[llvm] 8628e6d - [InstCombine] use freeze to enable poison-safe logic->select fold

[Sanjay Patel via llvm-commits]

Author: Sanjay Patel
Date: 2022-10-22T10:42:14-04:00
New Revision: 8628e6df70008e3eba86aee2a128b2e9b1f7cc26

URL: https://github.com/llvm/llvm-project/commit/8628e6df70008e3eba86aee2a128b2e9b1f7cc26
DIFF: https://github.com/llvm/llvm-project/commit/8628e6df70008e3eba86aee2a128b2e9b1f7cc26.diff

LOG: [InstCombine] use freeze to enable poison-safe logic->select fold

Without a freeze, this transform can leak poison to the output:
https://alive2.llvm.org/ce/z/GJuF9i

This makes the transform as uniform as possible, and it can help
reduce patterns like issue #58313 (although that particular
example probably still needs another transform).

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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
index 503d394c3f3c..6f21f703f0dc 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -2853,15 +2853,17 @@ Instruction *InstCombinerImpl::visitSelectInst(SelectInst &SI) {
       // (C && A) || (!C && B) --> sel C, A, B
       // (A && C) || (!C && B) --> sel C, A, B
       // (C && A) || (B && !C) --> sel C, A, B
-      // (A && C) || (B && !C) --> sel C, A, B (only with real 'and' ops)
+      // (A && C) || (B && !C) --> sel C, A, B (may require freeze)
       if (match(FalseVal, m_c_LogicalAnd(m_Not(m_Value(C)), m_Value(B))) &&
           match(CondVal, m_c_LogicalAnd(m_Specific(C), m_Value(A)))) {
         auto *SelCond = dyn_cast<SelectInst>(CondVal);
         auto *SelFVal = dyn_cast<SelectInst>(FalseVal);
-        if (!SelCond || !SelFVal ||
-            !match(SelFVal->getTrueValue(),
-                   m_Not(m_Specific(SelCond->getTrueValue()))))
-          return SelectInst::Create(C, A, B);
+        bool MayNeedFreeze = SelCond && SelFVal &&
+                             match(SelFVal->getTrueValue(),
+                                   m_Not(m_Specific(SelCond->getTrueValue())));
+        if (MayNeedFreeze)
+          C = Builder.CreateFreeze(C);
+        return SelectInst::Create(C, A, B);
       }
 
       // (!C && A) || (C && B) --> sel C, B, A

diff  --git a/llvm/test/Transforms/InstCombine/select-safe-transforms.ll b/llvm/test/Transforms/InstCombine/select-safe-transforms.ll
index 3eedfc0afaef..e92bac6abbba 100644
--- a/llvm/test/Transforms/InstCombine/select-safe-transforms.ll
+++ b/llvm/test/Transforms/InstCombine/select-safe-transforms.ll
@@ -529,14 +529,12 @@ define i1 @bools2_logical_commute2_and1_and2(i1 %a, i1 %c) {
   ret i1 %or
 }
 
-; This is not safe to transform if 'c' could be poison.
+; Freeze 'c' to prevent poison from leaking.
 
 define i1 @bools2_logical_commute3(i1 %a, i1 %b, i1 %c) {
 ; CHECK-LABEL: @bools2_logical_commute3(
-; CHECK-NEXT:    [[NOT:%.*]] = xor i1 [[C:%.*]], true
-; CHECK-NEXT:    [[AND1:%.*]] = select i1 [[A:%.*]], i1 [[C]], i1 false
-; CHECK-NEXT:    [[AND2:%.*]] = select i1 [[B:%.*]], i1 [[NOT]], i1 false
-; CHECK-NEXT:    [[OR:%.*]] = select i1 [[AND1]], i1 true, i1 [[AND2]]
+; CHECK-NEXT:    [[TMP1:%.*]] = freeze i1 [[C:%.*]]
+; CHECK-NEXT:    [[OR:%.*]] = select i1 [[TMP1]], i1 [[A:%.*]], i1 [[B:%.*]]
 ; CHECK-NEXT:    ret i1 [[OR]]
 ;
   %not = xor i1 %c, -1
@@ -546,6 +544,9 @@ define i1 @bools2_logical_commute3(i1 %a, i1 %b, i1 %c) {
   ret i1 %or
 }
 
+; No freeze needed when 'c' is guaranteed not be poison.
+; Intermediate logic folds may already reduce this.
+
 define i1 @bools2_logical_commute3_nopoison(i1 %a, i1 %b, i1 noundef %c) {
 ; CHECK-LABEL: @bools2_logical_commute3_nopoison(
 ; CHECK-NEXT:    [[OR:%.*]] = select i1 [[C:%.*]], i1 [[A:%.*]], i1 [[B:%.*]]