[NFC] CodeGen: Handle shift amount type in DAGTypeLegalizer::SplitInteger

[yaxunl]

Summary

This patch reverts change to X86TargetLowering::getScalarShiftAmountTy in
rL318727 and move the logic to DAGTypeLegalizer::SplitInteger.

The reason is that getScalarShiftAmountTy returns a shift amount type that
is suitable for common use cases in CodeGen. DAGTypeLegalizer::SplitInteger
is a rare situation which requires a shift amount type larger than what
getScalarShiftAmountTy. In this case, it is more reasonable to do special
handling of shift amount type in DAGTypeLegalizer::SplitInteger only. If
similar situations arises the logic may be moved to a separate function.

Since this is NFC, existing lit tests are sufficient.

[efriedma]

lib/CodeGen/SelectionDAG/LegalizeTypes.cpp

1175 ↗

(On Diff #123839)

Log2_32_Ceil?

[yaxunl]

lib/CodeGen/SelectionDAG/LegalizeTypes.cpp

1175 ↗

(On Diff #123839)

It is different. E.g, the number 32 requires 6 bits to hold, but Log2_32_Ceil(32) == 5.

[efriedma]

lib/CodeGen/SelectionDAG/LegalizeTypes.cpp

1175 ↗

(On Diff #123839)

Sure, but "32" isn't a legal shift amount for an i32.

Oh, wait, I see, you're not using the type I expected. I was expecting something like Log2_32_Ceil(Op.getValueType().getSizeInBits()). (I guess that's equivalent to what you wrote, but it's a bit more intuitive since Op.getValueType() is the type of the shift.)

[yaxunl]

Revised by Eli's comments.

lib/CodeGen/SelectionDAG/LegalizeTypes.cpp

1175 ↗

(On Diff #123839)

Revised. Thanks.

[efriedma]

LGTM

llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp

 /// Return the lower LoVT bits of Op in Lo and the upper HiVT bits in Hi.
 void DAGTypeLegalizer::SplitInteger(SDValue Op,
                                     EVT LoVT, EVT HiVT,
                                     SDValue &Lo, SDValue &Hi) {
   SDLoc dl(Op);
   assert(LoVT.getSizeInBits() + HiVT.getSizeInBits() ==
          Op.getValueSizeInBits() && "Invalid integer splitting!");
   Lo = DAG.getNode(ISD::TRUNCATE, dl, LoVT, Op);
-  Hi =
-      DAG.getNode(ISD::SRL, dl, Op.getValueType(), Op,
-                  DAG.getConstant(LoVT.getSizeInBits(), dl,
-                                  TLI.getScalarShiftAmountTy(
-                                      DAG.getDataLayout(), Op.getValueType())));
+  unsigned ReqShiftAmountInBits =
+      Log2_32_Ceil(Op.getValueType().getSizeInBits());
+  MVT ShiftAmountTy =
+      TLI.getScalarShiftAmountTy(DAG.getDataLayout(), Op.getValueType());
+  if (ReqShiftAmountInBits > ShiftAmountTy.getSizeInBits())
+    ShiftAmountTy = MVT::getIntegerVT(NextPowerOf2(ReqShiftAmountInBits));
+  Hi = DAG.getNode(ISD::SRL, dl, Op.getValueType(), Op,
+                   DAG.getConstant(LoVT.getSizeInBits(), dl, ShiftAmountTy));
   Hi = DAG.getNode(ISD::TRUNCATE, dl, HiVT, Hi);
 }
 
 /// Return the lower and upper halves of Op's bits in a value type half the
 /// size of Op's.
 void DAGTypeLegalizer::SplitInteger(SDValue Op,
                                     SDValue &Lo, SDValue &Hi) {
   EVT HalfVT =

llvm/trunk/lib/Target/X86/X86ISelLowering.h

 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_LIB_TARGET_X86_X86ISELLOWERING_H
 #define LLVM_LIB_TARGET_X86_X86ISELLOWERING_H
 
 #include "llvm/CodeGen/CallingConvLower.h"
 #include "llvm/CodeGen/SelectionDAG.h"
 #include "llvm/CodeGen/TargetLowering.h"
-#include "llvm/Support/MathExtras.h"
 #include "llvm/Target/TargetOptions.h"
 
 namespace llvm {
   class X86Subtarget;
   class X86TargetMachine;
 
   namespace X86ISD {
     // X86 Specific DAG Nodes
explicit X86TargetLowering(const X86TargetMachine &TM,
                                const X86Subtarget &STI);
 
     unsigned getJumpTableEncoding() const override;
     bool useSoftFloat() const override;
 
     void markLibCallAttributes(MachineFunction *MF, unsigned CC,
                                ArgListTy &Args) const override;
 
-    // For i512, DAGTypeLegalizer::SplitInteger needs a shift amount 256,
-    // which cannot be held by i8, therefore use i16 instead. In all the
-    // other situations i8 is sufficient.
     MVT getScalarShiftAmountTy(const DataLayout &, EVT VT) const override {
-      MVT T = VT.getSizeInBits() >= 512 ? MVT::i16 : MVT::i8;
-      assert((VT.getSizeInBits() + 1) / 2 < (1U << T.getSizeInBits()) &&
-             "Scalar shift amount type too small");
-      return T;
+      return MVT::i8;
     }
 
     const MCExpr *
     LowerCustomJumpTableEntry(const MachineJumpTableInfo *MJTI,
                               const MachineBasicBlock *MBB, unsigned uid,
                               MCContext &Ctx) const override;
 
     /// Returns relocation base for the given PIC jumptable.