[Thumb1] Any imm of i8 type on Thumb1 should have cost of 1

[zzheng]

Summary

A simple <MOVS rd, imm8> can materialize [-128, 127] in signed i8 type or [0, 255] in unsigned i8 type on Thumb1.

[zzheng]

ping...

[SjoerdMeijer]

I think the test file can use a bit of clean up: we don't the attributes, metadata, etc. But more importantly, can it perhaps be further reduced? Do we need all this code?

[SjoerdMeijer]

Ah, but looking a bit closer now, I am not sure this is the right thing to do. This changes makes any 8 bit value cheap, including negative numbers. And I am not sure if this is the right thing to do, since the Thumb1 immediates are positive numbers. It looks this a workaround for store-merging interacting badly with constant hoisting.

[dmgreen]

Is this because we can just use a MOVS and wont have to fill in any higher bits? And MOVS's aren't trivially rematerialisable? And Thumb2/Arm are handled by getT2SOImmVal?

So, yeah, looks sensible to me, and this does seem to reduce codesize. But please clean up the testcase.

[zzheng]

Cleaned up test case.

[zzheng]

In D52257#1241502, @SjoerdMeijer wrote:

Ah, but looking a bit closer now, I am not sure this is the right thing to do. This changes makes any 8 bit value cheap, including negative numbers. And I am not sure if this is the right thing to do, since the Thumb1 immediates are positive numbers. It looks this a workaround for store-merging interacting badly with constant hoisting.

Depends on how it's interpreted, 0xFF is -1 in signed i8 or 255 in unsigned i8.
Right, constant hoisting breaks store merging for this case.

In D52257#1241508, @dmgreen wrote:

Is this because we can just use a MOVS and wont have to fill in any higher bits? And MOVS's aren't trivially rematerialisable? And Thumb2/Arm are handled by getT2SOImmVal?

So, yeah, looks sensible to me, and this does seem to reduce codesize. But please clean up the testcase.

Yes, since it's i8 type, no need to deal with higher bits. We ran into this case with internal workload, it's intended to reduce code size.

[dmgreen]

Sounds good. I'm happy with this, if no one else has any issues.

[SjoerdMeijer]

Yep, sounds good, cheers.

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

if (Bits == 0 || Imm.getActiveBits() >= 64)
   }
   if (ST->isThumb2()) {
     if ((SImmVal >= 0 && SImmVal < 65536) ||
         (ARM_AM::getT2SOImmVal(ZImmVal) != -1) ||
         (ARM_AM::getT2SOImmVal(~ZImmVal) != -1))
       return 1;
     return ST->hasV6T2Ops() ? 2 : 3;
   }
-  // Thumb1.
-  if (SImmVal >= 0 && SImmVal < 256)
+  // Thumb1, any i8 imm cost 1.
+  if (Bits == 8 || (SImmVal >= 0 && SImmVal < 256))
     return 1;
   if ((~SImmVal < 256) || ARM_AM::isThumbImmShiftedVal(ZImmVal))
     return 2;
   // Load from constantpool.
   return 3;
 }
 
 // Constants smaller than 256 fit in the immediate field of

llvm/trunk/test/CodeGen/Thumb/consthoist-imm8-costs-1.ll

+; RUN: llc %s -o - | FileCheck %s
+
+target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+target triple = "thumbv6m-none-unknown-musleabi"
+
+@a = global i8 undef, align 4
+
+; Check that store-merging generates a single str i32 rather than strb+strb+strh,
+; i.e., -1 is not moved by constant-hoisting.
+; CHECK: movs [[R1:r[0-9]+]], #255
+; CHECK: lsls [[R2:r[0-9]+]], [[R1]], #16
+; CHECK: str  [[R2]]
+; CHECK: movs [[R3:r[0-9]+]], #255
+; CHECK: lsls [[R4:r[0-9]+]], [[R3]], #16
+; CHECK: str  [[R4]]
+; CHECK-NOT: strh
+; CHECK-NOT: strb
+
+define void @ham() {
+bb:
+  br i1 undef, label %bb1, label %bb2
+
+bb1:
+  store i8 0, i8* getelementptr inbounds (i8, i8* @a, i32 1), align 1
+  store i8 0, i8* getelementptr inbounds (i8, i8* @a, i32 0), align 4
+  store i8 -1, i8* getelementptr inbounds (i8, i8* @a, i32 2), align 2
+  store i8 0, i8* getelementptr inbounds (i8, i8* @a, i32 3), align 1
+  br label %bb3
+
+bb2:
+  store i8 0, i8* getelementptr inbounds (i8, i8* @a, i32 9), align 1
+  store i8 0, i8* getelementptr inbounds (i8, i8* @a, i32 8), align 4
+  store i8 -1, i8* getelementptr inbounds (i8, i8* @a, i32 10), align 2
+  store i8 0, i8* getelementptr inbounds (i8, i8* @a, i32 11), align 1
+  br label %bb3
+
+bb3:
+  ret void
+}