[llvm] a59cc5e - [InstCombine] Canonicalize select immediates

[David Green via llvm-commits]

Author: David Green
Date: 2019-12-19T12:36:46Z
New Revision: a59cc5e128f09ec5048c142cafaadea279406eba

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

LOG: [InstCombine] Canonicalize select immediates

In certain situations after inlining and simplification we end up with
code that is _almost_ a min/max pattern, but contains constants that
have been demand-bit optimised to the wrong values, ending up with code
like:
  %1 = icmp slt i32 %shr, -128
  %2 = select i1 %1, i32 128, i32 %shr
  %.inv = icmp sgt i32 %shr, 127
  %spec.select.i = select i1 %.inv, i32 127, i32 %2
  %conv7 = trunc i32 %spec.select.i to i8
This should be turned into a min/max pattern, but the -128 in the first
select was instead transformed into 128, as only the bottom byte was
ever demanded.

To fix this, I've put in further canonicalisation for the immediates of
selects, preferring to use the same value as the icmp if available.

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

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
    llvm/test/Transforms/InstCombine/select-imm-canon.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
index 359fc55035fa..47ce83974c8d 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
@@ -350,8 +350,36 @@ Value *InstCombiner::SimplifyDemandedUseBits(Value *V, APInt DemandedMask,
     assert(!LHSKnown.hasConflict() && "Bits known to be one AND zero?");
 
     // If the operands are constants, see if we can simplify them.
-    if (ShrinkDemandedConstant(I, 1, DemandedMask) ||
-        ShrinkDemandedConstant(I, 2, DemandedMask))
+    // This is similar to ShrinkDemandedConstant, but for a select we want to
+    // try to keep the selected constants the same as icmp value constants, if
+    // we can. This helps not break apart (or helps put back together)
+    // canonical patterns like min and max.
+    auto CanonicalizeSelectConstant = [](Instruction *I, unsigned OpNo,
+                                         APInt DemandedMask) {
+      const APInt *SelC;
+      if (!match(I->getOperand(OpNo), m_APInt(SelC)))
+        return false;
+
+      // Get the constant out of the ICmp, if there is one.
+      const APInt *CmpC;
+      ICmpInst::Predicate Pred;
+      if (!match(I->getOperand(0), m_c_ICmp(Pred, m_APInt(CmpC), m_Value())) ||
+          CmpC->getBitWidth() != SelC->getBitWidth())
+        return ShrinkDemandedConstant(I, OpNo, DemandedMask);
+
+      // If the constant is already the same as the ICmp, leave it as-is.
+      if (*CmpC == *SelC)
+        return false;
+      // If the constants are not already the same, but can be with the demand
+      // mask, use the constant value from the ICmp.
+      if ((*CmpC & DemandedMask) == (*SelC & DemandedMask)) {
+        I->setOperand(OpNo, ConstantInt::get(I->getType(), *CmpC));
+        return true;
+      }
+      return ShrinkDemandedConstant(I, OpNo, DemandedMask);
+    };
+    if (CanonicalizeSelectConstant(I, 1, DemandedMask) ||
+        CanonicalizeSelectConstant(I, 2, DemandedMask))
       return I;
 
     // Only known if known in both the LHS and RHS.

diff  --git a/llvm/test/Transforms/InstCombine/select-imm-canon.ll b/llvm/test/Transforms/InstCombine/select-imm-canon.ll
index 1633512b70e2..b00d9e3f2565 100644
--- a/llvm/test/Transforms/InstCombine/select-imm-canon.ll
+++ b/llvm/test/Transforms/InstCombine/select-imm-canon.ll
@@ -4,8 +4,8 @@
 define i8 @single(i32 %A) {
 ; CHECK-LABEL: @single(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[L1:%.*]] = icmp slt i32 [[A:%.*]], -128
-; CHECK-NEXT:    [[L2:%.*]] = select i1 [[L1]], i32 128, i32 [[A]]
+; CHECK-NEXT:    [[TMP0:%.*]] = icmp sgt i32 [[A:%.*]], -128
+; CHECK-NEXT:    [[L2:%.*]] = select i1 [[TMP0]], i32 [[A]], i32 -128
 ; CHECK-NEXT:    [[CONV7:%.*]] = trunc i32 [[L2]] to i8
 ; CHECK-NEXT:    ret i8 [[CONV7]]
 ;
@@ -19,10 +19,10 @@ entry:
 define i8 @double(i32 %A) {
 ; CHECK-LABEL: @double(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[L1:%.*]] = icmp slt i32 [[A:%.*]], -128
-; CHECK-NEXT:    [[L2:%.*]] = select i1 [[L1]], i32 128, i32 [[A]]
-; CHECK-NEXT:    [[DOTINV:%.*]] = icmp sgt i32 [[A]], 127
-; CHECK-NEXT:    [[SPEC_SELECT_I:%.*]] = select i1 [[DOTINV]], i32 127, i32 [[L2]]
+; CHECK-NEXT:    [[TMP0:%.*]] = icmp sgt i32 [[A:%.*]], -128
+; CHECK-NEXT:    [[L2:%.*]] = select i1 [[TMP0]], i32 [[A]], i32 -128
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp slt i32 [[L2]], 127
+; CHECK-NEXT:    [[SPEC_SELECT_I:%.*]] = select i1 [[TMP1]], i32 [[L2]], i32 127
 ; CHECK-NEXT:    [[CONV7:%.*]] = trunc i32 [[SPEC_SELECT_I]] to i8
 ; CHECK-NEXT:    ret i8 [[CONV7]]
 ;
@@ -39,7 +39,7 @@ define i8 @thisdoesnotloop(i32 %A, i32 %B) {
 ; CHECK-LABEL: @thisdoesnotloop(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[L1:%.*]] = icmp slt i32 [[A:%.*]], -128
-; CHECK-NEXT:    [[L2:%.*]] = select i1 [[L1]], i32 128, i32 [[B:%.*]]
+; CHECK-NEXT:    [[L2:%.*]] = select i1 [[L1]], i32 -128, i32 [[B:%.*]]
 ; CHECK-NEXT:    [[CONV7:%.*]] = trunc i32 [[L2]] to i8
 ; CHECK-NEXT:    ret i8 [[CONV7]]
 ;
@@ -52,10 +52,10 @@ entry:
 
 define i8 @original(i32 %A, i32 %B) {
 ; CHECK-LABEL: @original(
-; CHECK-NEXT:    [[TMP1:%.*]] = icmp slt i32 [[A:%.*]], -128
-; CHECK-NEXT:    [[TMP2:%.*]] = select i1 [[TMP1]], i32 128, i32 [[A]]
-; CHECK-NEXT:    [[DOTINV:%.*]] = icmp sgt i32 [[A]], 127
-; CHECK-NEXT:    [[SPEC_SELECT_I:%.*]] = select i1 [[DOTINV]], i32 127, i32 [[TMP2]]
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp sgt i32 [[A:%.*]], -128
+; CHECK-NEXT:    [[TMP2:%.*]] = select i1 [[TMP1]], i32 [[A]], i32 -128
+; CHECK-NEXT:    [[TMP3:%.*]] = icmp slt i32 [[TMP2]], 127
+; CHECK-NEXT:    [[SPEC_SELECT_I:%.*]] = select i1 [[TMP3]], i32 [[TMP2]], i32 127
 ; CHECK-NEXT:    [[CONV7:%.*]] = trunc i32 [[SPEC_SELECT_I]] to i8
 ; CHECK-NEXT:    ret i8 [[CONV7]]
 ;