[PATCH] D84664: [InstCombine] Fold shift-select if all the operands are constant

[Matteo Favaro via Phabricator via llvm-commits]

fvrmatteo updated this revision to Diff 281868.
fvrmatteo added a comment.

@nikic I tried to oblige to your comment and come up with a check in **SimplifySelectsFeedingBinaryOp** to verify if we are in the condition where either the instruction has one use or True and False fold into constants.

I'm not sure I understood the question about the addition of the test file. Technically this is an issue that I noticed when **%v1** is ashr/lshr/sub, but for example the same case with add/xor/or/shl properly folds with no modifications whatsoever. Hence my original confusion and the test that we carried on with @lebedev.ri about the **hasOneUse** call part of the **canEvaluateShifted** function.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84664/new/

https://reviews.llvm.org/D84664

Files:
  llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
  llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
  llvm/test/Transforms/InstCombine/lshr-select-const.ll


Index: llvm/test/Transforms/InstCombine/lshr-select-const.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/InstCombine/lshr-select-const.ll
@@ -0,0 +1,23 @@
+; RUN: opt < %s -instcombine -S | FileCheck %s
+
+; Instcombine should be able to eliminate the 'lshr' shift because
+; the 'select' uses constant source operands that will be shifted
+; by a constant amount.
+
+define i64 @fold_lshr(i1 %arg) {
+; CHECK-LABEL: @fold_lshr(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[V8:%.*]] = select i1 [[ARG:%.*]], i64 5369111361, i64 5369111591
+; CHECK-NEXT:    ret i64 [[V8]]
+entry:
+  %v0 = select i1 %arg, i64 64, i64 0
+  %v1 = lshr exact i64 %v0, 6
+  %v2 = xor i64 %v1, 1
+  %v3 = shl nuw i64 %v0, 57
+  %v4 = ashr exact i64 %v3, 63
+  %v5 = sub nsw i64 0, %v2
+  %v6 = and i64 %v5, 5369111591
+  %v7 = and i64 %v4, 5369111361
+  %v8 = add nuw nsw i64 %v7, %v6
+  ret i64 %v8
+}
Index: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
===================================================================
--- llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -843,16 +843,28 @@
       else if (True && !False)
         False = Builder.CreateBinOp(Opcode, C, F);
     }
-  } else if (LHSIsSelect && LHS->hasOneUse()) {
+  } else if (LHSIsSelect) {
     // (A ? B : C) op Y -> A ? (B op Y) : (C op Y)
-    Cond = A;
-    True = SimplifyBinOp(Opcode, B, RHS, FMF, Q);
-    False = SimplifyBinOp(Opcode, C, RHS, FMF, Q);
-  } else if (RHSIsSelect && RHS->hasOneUse()) {
+    auto *TempTrue = SimplifyBinOp(Opcode, B, RHS, FMF, Q);
+    auto *TempFalse = SimplifyBinOp(Opcode, C, RHS, FMF, Q);
+    // We optimise single uses and folding into constants
+    if (LHS->hasOneUse() || (isa_and_nonnull<ConstantInt>(TempTrue) &&
+                             isa_and_nonnull<ConstantInt>(TempFalse))) {
+      Cond = A;
+      True = TempTrue;
+      False = TempFalse;
+    }
+  } else if (RHSIsSelect) {
     // X op (D ? E : F) -> D ? (X op E) : (X op F)
-    Cond = D;
-    True = SimplifyBinOp(Opcode, LHS, E, FMF, Q);
-    False = SimplifyBinOp(Opcode, LHS, F, FMF, Q);
+    auto *TempTrue = SimplifyBinOp(Opcode, LHS, E, FMF, Q);
+    auto *TempFalse = SimplifyBinOp(Opcode, LHS, F, FMF, Q);
+    // We optimise single uses and folding into constants
+    if (RHS->hasOneUse() || (isa_and_nonnull<ConstantInt>(TempTrue) &&
+                             isa_and_nonnull<ConstantInt>(TempFalse))) {
+      Cond = D;
+      True = TempTrue;
+      False = TempFalse;
+    }
   }
 
   if (!True || !False)
Index: llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
===================================================================
--- llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
+++ llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
@@ -386,6 +386,9 @@
     if (Instruction *Res = FoldShiftByConstant(Op0, CUI, I))
       return Res;
 
+  if (Value *V = SimplifySelectsFeedingBinaryOp(I, Op0, Op1))
+    return replaceInstUsesWith(I, V);
+
   if (auto *NewShift = cast_or_null<Instruction>(
           reassociateShiftAmtsOfTwoSameDirectionShifts(&I, SQ)))
     return NewShift;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D84664.281868.patch
Type: text/x-patch
Size: 3231 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200730/0c4dd4e2/attachment.bin>