[PATCH] D69116: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (1/2)

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/TargetLowering.h:2614
+  virtual unsigned getShiftAmountThreshold(EVT VT) const {
+    return VT.getSizeInBits();
+  }
----------------
It may not be used with vector types currently, but it would be safer to use:
  VT.getScalarSizeInBits()


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:3622
+          if (AndRHS->getAPIntValue().isPowerOf2() &&
+                ShCt <= TLI.getShiftAmountThreshold(ShiftTy)) {
             return DAG.getNode(ISD::TRUNCATE, dl, VT,
----------------
This is probably not working as you expected because the shift type may not be the same as the value type.
I added an x86 test to demonstrate:
rL375158


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:3633-3634
             return DAG.getNode(ISD::TRUNCATE, dl, VT,
                                DAG.getNode(ISD::SRL, dl, N0.getValueType(), N0,
-                                      DAG.getConstant(C1.logBase2(), dl,
-                                                      ShiftTy)));
+                                      DAG.getConstant(ShCt, dl, ShiftTy)));
           }
----------------
Here and above the indentation looks non-standard. Use "clang-format" to correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69116