[PATCH] D69326: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (3)

[Joan LLuch via Phabricator via llvm-commits]

joanlluch created this revision.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.
joanlluch added reviewers: spatel, lebedev.ri, asl.

Additional filtering of undesired shifts for targets that do not support them efficiently.

Related with  D69116 <https://reviews.llvm.org/D69116> and  D69120 <https://reviews.llvm.org/D69120>

Applies the TLI.getShiftAmountThreshold hook to prevent undesired generation of shifts for the following IR code:

  define i16 @testShiftBits(i16 %a) {
  entry:
    %and = and i16 %a, -64
    %cmp = icmp eq i16 %and, 64
    %conv = zext i1 %cmp to i16
    ret i16 %conv
  }
  
  define i16 @testShiftBits_11(i16 %a) {
  entry:
    %cmp = icmp ugt i16 %a, 63
    %conv = zext i1 %cmp to i16
    ret i16 %conv
  }
  
  define i16 @testShiftBits_12(i16 %a) {
  entry:
    %cmp = icmp ult i16 %a, 64
    %conv = zext i1 %cmp to i16
    ret i16 %conv
  }

The attached diff file shows the piece code in TargetLowering that is responsible for the generation of shifts in relation to the IR above.

For targets with restricted legal immediates, before applying this patch, shifts will be generated as an attempt to avoid such immediates. However, shifts may be undesired if they are even more expensive for the target.

In this case, the MSP430 trunk target is not suitable for tests because it does not implement "isLegalICmpImmediate". The default implementation returns always true, so the TargetLowering code that would generate shifts is never executed anyway. Therefore, the effect of this patch can't be shown as differential test files for the MSP430.

If desired, the differential effect of this patch can be shown for the MSP430 by previously implementing "isLegalICmpImmediate" to return false for large immediates. However, I think there's little point on doing so because, for this target, it is already correct that isLegalICmpImmediate returns always true.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69326

Files:
  llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp


Index: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -3646,15 +3646,17 @@
           const APInt &AndRHSC = AndRHS->getAPIntValue();
           if ((-AndRHSC).isPowerOf2() && (AndRHSC & C1) == C1) {
             unsigned ShiftBits = AndRHSC.countTrailingZeros();
-            auto &DL = DAG.getDataLayout();
-            EVT ShiftTy = getShiftAmountTy(N0.getValueType(), DL,
-                                           !DCI.isBeforeLegalize());
-            EVT CmpTy = N0.getValueType();
-            SDValue Shift = DAG.getNode(ISD::SRL, dl, CmpTy, N0.getOperand(0),
-                                        DAG.getConstant(ShiftBits, dl,
-                                                        ShiftTy));
-            SDValue CmpRHS = DAG.getConstant(C1.lshr(ShiftBits), dl, CmpTy);
-            return DAG.getSetCC(dl, VT, Shift, CmpRHS, Cond);
+            if (ShiftBits <= TLI.getShiftAmountThreshold(ShValTy)) {
+              auto &DL = DAG.getDataLayout();
+              EVT ShiftTy = getShiftAmountTy(ShValTy, DL,
+                                            !DCI.isBeforeLegalize());
+              EVT CmpTy = ShValTy;
+              SDValue Shift = DAG.getNode(ISD::SRL, dl, CmpTy, N0.getOperand(0),
+                                          DAG.getConstant(ShiftBits, dl,
+                                                          ShiftTy));
+              SDValue CmpRHS = DAG.getConstant(C1.lshr(ShiftBits), dl, CmpTy);
+              return DAG.getSetCC(dl, VT, Shift, CmpRHS, Cond);
+            }
           }
         }
       } else if (Cond == ISD::SETULT || Cond == ISD::SETUGE ||
@@ -3676,11 +3678,12 @@
         }
         NewC.lshrInPlace(ShiftBits);
         if (ShiftBits && NewC.getMinSignedBits() <= 64 &&
-          isLegalICmpImmediate(NewC.getSExtValue())) {
+            isLegalICmpImmediate(NewC.getSExtValue()) &&
+            ShiftBits <= TLI.getShiftAmountThreshold(ShValTy)) {
           auto &DL = DAG.getDataLayout();
-          EVT ShiftTy = getShiftAmountTy(N0.getValueType(), DL,
+          EVT ShiftTy = getShiftAmountTy(ShValTy, DL,
                                          !DCI.isBeforeLegalize());
-          EVT CmpTy = N0.getValueType();
+          EVT CmpTy = ShValTy;
           SDValue Shift = DAG.getNode(ISD::SRL, dl, CmpTy, N0,
                                       DAG.getConstant(ShiftBits, dl, ShiftTy));
           SDValue CmpRHS = DAG.getConstant(NewC, dl, CmpTy);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D69326.226082.patch
Type: text/x-patch
Size: 2611 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191022/117ebd74/attachment.bin>