[ARM] Fix cost computation for constant hoisting

[weimingz]

Summary

Bug: https://llvm.org/bugs/show_bug.cgi?id=28282

Currently the cost model of constant hoisting checks the bit width of the data type of the constants.
However, the actual immediate value is small enough and not need to be hoisted.
This patch checks for the actual bit width needed for the constant.

[weimingz]

using ">=" because when convert to signed values, getMinSignedBits() may return 65. Instead of checking both activeBits and MinSignedBits(), we just check if activeBits() is no larger than 64.

[rengolin]

Hi Weiming,

The code makes sense, but the test is not clear as to what it's all about or what's the expected output, which will make it very hard for people to follow up when it breaks for other reasons.

Can you add a comment to the test explaining what's this is about and what's the expected behaviour?

cheers,
--renato

test/Transforms/ConstantHoisting/ARM/bad-cases.ll

107 ↗

(On Diff #61811)

Was this line supposed to be a CHECK?

[weimingz]

Hi Renato,

Thanks for the feedback.
I Simplified the test case and added comments.

[rengolin]

LGTM, thanks!

llvm/trunk/lib/Target/ARM/ARMTargetTransformInfo.cpp

 using namespace llvm;
 
 #define DEBUG_TYPE "armtti"
 
 int ARMTTIImpl::getIntImmCost(const APInt &Imm, Type *Ty) {
   assert(Ty->isIntegerTy());
 
  unsigned Bits = Ty->getPrimitiveSizeInBits();
- if (Bits == 0 || Bits > 64)
+ if (Bits == 0 || Imm.getActiveBits() >= 64)
    return 4;
 
   int64_t SImmVal = Imm.getSExtValue();
   uint64_t ZImmVal = Imm.getZExtValue();
   if (!ST->isThumb()) {
     if ((SImmVal >= 0 && SImmVal < 65536) ||
         (ARM_AM::getSOImmVal(ZImmVal) != -1) ||
         (ARM_AM::getSOImmVal(~ZImmVal) != -1))

llvm/trunk/test/Transforms/ConstantHoisting/ARM/bad-cases.ll

loop:
   %res2 = urem i32 %l2, 1000000000
   store volatile i32 %res2, i32* %addr
   %again = icmp eq i32 %res1, %res2
   br i1 %again, label %loop, label %end
 
 end:
   ret void
 }
+
+;PR 28282: even when data type is larger than 64-bit, the bit width of the
+;constant operand could be smaller than 64-bit. In this case, there is no
+;benefit to hoist the constant.
+define i32 @struct_type_test(i96 %a0, i96 %a1) {
+;CHECK-LABEL: @struct_type_test
+entry:
+;CHECK-NOT: %const = bitcast i96 32 to i96
+;CHECK: lshr0 = lshr i96 %a0, 32
+  %lshr0 = lshr i96 %a0, 32
+  %cast0 = trunc i96 %lshr0 to i32
+;CHECK: lshr1 = lshr i96 %a1, 32
+  %lshr1 = lshr i96 %a1, 32
+  %cast1 = trunc i96 %lshr1 to i32
+  %ret = add i32 %cast0, %cast1
+  ret i32 %ret
+}