[ARM] Use sub for negative offset load/store in thumb1

[dmgreen]

Summary

This attempts to optimise negative values used in load/store operands
a little. We currently try to selct them as rr, materialising the
negative constant using a MOV/MVN pair. This instead selects ri with
an immediate of 0, forcing the add node to become a simpler sub.

The ldrsb and ldrsh tests here are not really better, but not worse either.
It will depend upon whether the constant or the add havee multiple uses.
It appears to be fairly common for an address to be used, and stored
somewhere at the same time.

[efriedma]

Can you give an example where this transform helps for ldrsb/ldrsh?

lib/Target/ARM/ARMISelDAGToDAG.cpp

1043 ↗

(On Diff #183184)

Does the actual value matter here? If we're going to generate a constant pool load, this doesn't seem profitable.

[dmgreen]

I've removed the ldrsh/ldrsb and made sure the value is a legal for a sub. This may or may not be what you meant, let me know.

[efriedma]

LGTM

lib/Target/ARM/ARMInstrThumb.td

190 ↗

(On Diff #183846)

Maybe add a brief comment here noting this is separate because we use different heuristics for ldrsb/ldrsh.

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

public:
   bool SelectAddrMode5FP16(SDValue N, SDValue &Base, SDValue &Offset);
   bool SelectAddrMode6(SDNode *Parent, SDValue N, SDValue &Addr,SDValue &Align);
   bool SelectAddrMode6Offset(SDNode *Op, SDValue N, SDValue &Offset);
 
   bool SelectAddrModePC(SDValue N, SDValue &Offset, SDValue &Label);
 
   // Thumb Addressing Modes:
   bool SelectThumbAddrModeRR(SDValue N, SDValue &Base, SDValue &Offset);
+  bool SelectThumbAddrModeRRSext(SDValue N, SDValue &Base, SDValue &Offset);
   bool SelectThumbAddrModeImm5S(SDValue N, unsigned Scale, SDValue &Base,
                                 SDValue &OffImm);
   bool SelectThumbAddrModeImm5S1(SDValue N, SDValue &Base,
                                  SDValue &OffImm);
   bool SelectThumbAddrModeImm5S2(SDValue N, SDValue &Base,
                                  SDValue &OffImm);
   bool SelectThumbAddrModeImm5S4(SDValue N, SDValue &Base,
                                  SDValue &OffImm);
bool ARMDAGToDAGISel::SelectAddrModePC(SDValue N,
   return false;
 }
 
 
 //===----------------------------------------------------------------------===//
 //                         Thumb Addressing Modes
 //===----------------------------------------------------------------------===//
 
-bool ARMDAGToDAGISel::SelectThumbAddrModeRR(SDValue N,
-                                            SDValue &Base, SDValue &Offset){
+static bool shouldUseZeroOffsetLdSt(SDValue N) {
+  // Negative numbers are difficult to materialise in thumb1. If we are
+  // selecting the add of a negative, instead try to select ri with a zero
+  // offset, so create the add node directly which will become a sub.
+  if (N.getOpcode() != ISD::ADD)
+    return false;
+
+  // Look for an imm which is not legal for ld/st, but is legal for sub.
+  if (auto C = dyn_cast<ConstantSDNode>(N.getOperand(1)))
+    return C->getSExtValue() < 0 && C->getSExtValue() >= -255;
+
+  return false;
+}
+
+bool ARMDAGToDAGISel::SelectThumbAddrModeRRSext(SDValue N, SDValue &Base,
+                                                SDValue &Offset) {
   if (N.getOpcode() != ISD::ADD && !CurDAG->isBaseWithConstantOffset(N)) {
     ConstantSDNode *NC = dyn_cast<ConstantSDNode>(N);
     if (!NC || !NC->isNullValue())
       return false;
 
     Base = Offset = N;
     return true;
   }
 
   Base = N.getOperand(0);
   Offset = N.getOperand(1);
   return true;
 }
 
+bool ARMDAGToDAGISel::SelectThumbAddrModeRR(SDValue N, SDValue &Base,
+                                            SDValue &Offset) {
+  if (shouldUseZeroOffsetLdSt(N))
+    return false; // Select ri instead
+  return SelectThumbAddrModeRRSext(N, Base, Offset);
+}
+
 bool
 ARMDAGToDAGISel::SelectThumbAddrModeImm5S(SDValue N, unsigned Scale,
                                           SDValue &Base, SDValue &OffImm) {
+  if (shouldUseZeroOffsetLdSt(N)) {
+    Base = N;
+    OffImm = CurDAG->getTargetConstant(0, SDLoc(N), MVT::i32);
+    return true;
+  }
+
   if (!CurDAG->isBaseWithConstantOffset(N)) {
     if (N.getOpcode() == ISD::ADD) {
       return false; // We want to select register offset instead
     } else if (N.getOpcode() == ARMISD::Wrapper &&
         N.getOperand(0).getOpcode() != ISD::TargetGlobalAddress &&
         N.getOperand(0).getOpcode() != ISD::TargetExternalSymbol &&
         N.getOperand(0).getOpcode() != ISD::TargetConstantPool &&
         N.getOperand(0).getOpcode() != ISD::TargetGlobalTLSAddress) {

llvm/trunk/lib/Target/ARM/ARMInstrThumb.td

def t_addrmode_rr : MemOperand,
                     ComplexPattern<i32, 2, "SelectThumbAddrModeRR", []> {
   let EncoderMethod = "getThumbAddrModeRegRegOpValue";
   let PrintMethod = "printThumbAddrModeRROperand";
   let DecoderMethod = "DecodeThumbAddrModeRR";
   let ParserMatchClass = t_addrmode_rr_asm_operand;
   let MIOperandInfo = (ops tGPR:$base, tGPR:$offsreg);
 }
 
+// t_addrmode_rr_sext := reg + reg
+//
+// This is similar to t_addrmode_rr, but uses different heuristics for
+// ldrsb/ldrsh.
+def t_addrmode_rr_sext : MemOperand,
+                    ComplexPattern<i32, 2, "SelectThumbAddrModeRRSext", []> {
+  let EncoderMethod = "getThumbAddrModeRegRegOpValue";
+  let PrintMethod = "printThumbAddrModeRROperand";
+  let DecoderMethod = "DecodeThumbAddrModeRR";
+  let ParserMatchClass = t_addrmode_rr_asm_operand;
+  let MIOperandInfo = (ops tGPR:$base, tGPR:$offsreg);
+}
+
 // t_addrmode_rrs := reg + reg
 //
 // We use separate scaled versions because the Select* functions need
 // to explicitly check for a matching constant and return false here so that
 // the reg+imm forms will match instead. This is a horrible way to do that,
 // as it forces tight coupling between the methods, but it's how selectiondag
 // currently works.
 def t_addrmode_rrs1 : MemOperand,

 // A8.6.76 & A8.6.73
 defm tLDRH : thumb_ld_rr_ri_enc<0b101, 0b1000, t_addrmode_rr,
                                 t_addrmode_is2, AddrModeT1_2,
                                 IIC_iLoad_bh_r, IIC_iLoad_bh_i, "ldrh",
                                 zextloadi16>;
 
 let AddedComplexity = 10 in
 def tLDRSB :                    // A8.6.80
-  T1pILdStEncode<0b011, (outs tGPR:$Rt), (ins t_addrmode_rr:$addr),
+  T1pILdStEncode<0b011, (outs tGPR:$Rt), (ins t_addrmode_rr_sext:$addr),
                  AddrModeT1_1, IIC_iLoad_bh_r,
                  "ldrsb", "\t$Rt, $addr",
-                 [(set tGPR:$Rt, (sextloadi8 t_addrmode_rr:$addr))]>;
+                 [(set tGPR:$Rt, (sextloadi8 t_addrmode_rr_sext:$addr))]>;
 
 let AddedComplexity = 10 in
 def tLDRSH :                    // A8.6.84
-  T1pILdStEncode<0b111, (outs tGPR:$Rt), (ins t_addrmode_rr:$addr),
+  T1pILdStEncode<0b111, (outs tGPR:$Rt), (ins t_addrmode_rr_sext:$addr),
                  AddrModeT1_2, IIC_iLoad_bh_r,
                  "ldrsh", "\t$Rt, $addr",
-                 [(set tGPR:$Rt, (sextloadi16 t_addrmode_rr:$addr))]>;
+                 [(set tGPR:$Rt, (sextloadi16 t_addrmode_rr_sext:$addr))]>;
 
 
 def tSTRspi : T1pIs<(outs), (ins tGPR:$Rt, t_addrmode_sp:$addr), IIC_iStore_i,
                     "str", "\t$Rt, $addr",
                     [(store tGPR:$Rt, t_addrmode_sp:$addr)]>,
               T1LdStSP<{0,?,?}> {
   bits<3> Rt;
   bits<8> addr;

llvm/trunk/test/CodeGen/ARM/load.ll

entry:
   %1 = load i16, i16* %0, align 2
   %conv = sext i16 %1 to i32
   ret i32 %conv
 }
 
 define i32 @ldrb_ri_negative(i8* %p) {
 ; CHECK-T1-LABEL: ldrb_ri_negative:
 ; CHECK-T1:       @ %bb.0: @ %entry
-; CHECK-T1-NEXT:    movs r1, #0
-; CHECK-T1-NEXT:    mvns r1, r1
-; CHECK-T1-NEXT:    ldrb r0, [r0, r1]
+; CHECK-T1-NEXT:    subs r0, r0, #1
+; CHECK-T1-NEXT:    ldrb r0, [r0]
 ; CHECK-T1-NEXT:    bx lr
 ;
 ; CHECK-T2-LABEL: ldrb_ri_negative:
 ; CHECK-T2:       @ %bb.0: @ %entry
 ; CHECK-T2-NEXT:    ldrb r0, [r0, #-1]
 ; CHECK-T2-NEXT:    bx lr
 entry:
   %add.ptr = getelementptr inbounds i8, i8* %p, i32 -1
   %0 = load i8, i8* %add.ptr, align 1
   %conv = zext i8 %0 to i32
   ret i32 %conv
 }
 
 define i32 @ldrh_ri_negative(i8* %p) {
 ; CHECK-T1-LABEL: ldrh_ri_negative:
 ; CHECK-T1:       @ %bb.0: @ %entry
-; CHECK-T1-NEXT:    movs r1, #0
-; CHECK-T1-NEXT:    mvns r1, r1
-; CHECK-T1-NEXT:    ldrh r0, [r0, r1]
+; CHECK-T1-NEXT:    subs r0, r0, #1
+; CHECK-T1-NEXT:    ldrh r0, [r0]
 ; CHECK-T1-NEXT:    bx lr
 ;
 ; CHECK-T2-LABEL: ldrh_ri_negative:
 ; CHECK-T2:       @ %bb.0: @ %entry
 ; CHECK-T2-NEXT:    ldrh r0, [r0, #-1]
 ; CHECK-T2-NEXT:    bx lr
 entry:
   %add.ptr = getelementptr inbounds i8, i8* %p, i32 -1
   %0 = bitcast i8* %add.ptr to i16*
   %1 = load i16, i16* %0, align 2
   %conv = zext i16 %1 to i32
   ret i32 %conv
 }
 
 define i32 @ldr_ri_negative(i8* %p) {
 ; CHECK-T1-LABEL: ldr_ri_negative:
 ; CHECK-T1:       @ %bb.0: @ %entry
-; CHECK-T1-NEXT:    movs r1, #0
-; CHECK-T1-NEXT:    mvns r1, r1
-; CHECK-T1-NEXT:    ldr r0, [r0, r1]
+; CHECK-T1-NEXT:    subs r0, r0, #1
+; CHECK-T1-NEXT:    ldr r0, [r0]
 ; CHECK-T1-NEXT:    bx lr
 ;
 ; CHECK-T2-LABEL: ldr_ri_negative:
 ; CHECK-T2:       @ %bb.0: @ %entry
 ; CHECK-T2-NEXT:    ldr r0, [r0, #-1]
 ; CHECK-T2-NEXT:    bx lr
 entry:
   %add.ptr = getelementptr inbounds i8, i8* %p, i32 -1
   %0 = bitcast i8* %add.ptr to i32*
   %1 = load i32, i32* %0, align 4
   ret i32 %1
 }
 
 define void @strb_ri_negative(i8* %p, i32 %x) {
 ; CHECK-T1-LABEL: strb_ri_negative:
 ; CHECK-T1:       @ %bb.0: @ %entry
-; CHECK-T1-NEXT:    movs r2, #0
-; CHECK-T1-NEXT:    mvns r2, r2
-; CHECK-T1-NEXT:    strb r1, [r0, r2]
+; CHECK-T1-NEXT:    subs r0, r0, #1
+; CHECK-T1-NEXT:    strb r1, [r0]
 ; CHECK-T1-NEXT:    bx lr
 ;
 ; CHECK-T2-LABEL: strb_ri_negative:
 ; CHECK-T2:       @ %bb.0: @ %entry
 ; CHECK-T2-NEXT:    strb r1, [r0, #-1]
 ; CHECK-T2-NEXT:    bx lr
 entry:
   %conv = trunc i32 %x to i8
   %add.ptr = getelementptr inbounds i8, i8* %p, i32 -1
   store i8 %conv, i8* %add.ptr, align 1
   ret void
 }
 
 define void @strh_ri_negative(i8* %p, i32 %x) {
 ; CHECK-T1-LABEL: strh_ri_negative:
 ; CHECK-T1:       @ %bb.0: @ %entry
-; CHECK-T1-NEXT:    movs r2, #0
-; CHECK-T1-NEXT:    mvns r2, r2
-; CHECK-T1-NEXT:    strh r1, [r0, r2]
+; CHECK-T1-NEXT:    subs r0, r0, #1
+; CHECK-T1-NEXT:    strh r1, [r0]
 ; CHECK-T1-NEXT:    bx lr
 ;
 ; CHECK-T2-LABEL: strh_ri_negative:
 ; CHECK-T2:       @ %bb.0: @ %entry
 ; CHECK-T2-NEXT:    strh r1, [r0, #-1]
 ; CHECK-T2-NEXT:    bx lr
 entry:
   %conv = trunc i32 %x to i16
   %add.ptr = getelementptr inbounds i8, i8* %p, i32 -1
   %0 = bitcast i8* %add.ptr to i16*
   store i16 %conv, i16* %0, align 2
   ret void
 }
 
 define void @str_ri_negative(i8* %p, i32 %x) {
 ; CHECK-T1-LABEL: str_ri_negative:
 ; CHECK-T1:       @ %bb.0: @ %entry
-; CHECK-T1-NEXT:    movs r2, #0
-; CHECK-T1-NEXT:    mvns r2, r2
-; CHECK-T1-NEXT:    str r1, [r0, r2]
+; CHECK-T1-NEXT:    subs r0, r0, #1
+; CHECK-T1-NEXT:    str r1, [r0]
 ; CHECK-T1-NEXT:    bx lr
 ;
 ; CHECK-T2-LABEL: str_ri_negative:
 ; CHECK-T2:       @ %bb.0: @ %entry
 ; CHECK-T2-NEXT:    str r1, [r0, #-1]
 ; CHECK-T2-NEXT:    bx lr
 entry:
   %add.ptr = getelementptr inbounds i8, i8* %p, i32 -1
entry:
   %1 = load i16, i16* %0, align 2
   %conv = sext i16 %1 to i32
   ret i32 %conv
 }
 
 define i32 @ldrb_ri_negative255(i8* %p) {
 ; CHECK-T1-LABEL: ldrb_ri_negative255:
 ; CHECK-T1:       @ %bb.0: @ %entry
-; CHECK-T1-NEXT:    movs r1, #254
-; CHECK-T1-NEXT:    mvns r1, r1
-; CHECK-T1-NEXT:    ldrb r0, [r0, r1]
+; CHECK-T1-NEXT:    subs r0, #255
+; CHECK-T1-NEXT:    ldrb r0, [r0]
 ; CHECK-T1-NEXT:    bx lr
 ;
 ; CHECK-T2-LABEL: ldrb_ri_negative255:
 ; CHECK-T2:       @ %bb.0: @ %entry
 ; CHECK-T2-NEXT:    ldrb r0, [r0, #-255]
 ; CHECK-T2-NEXT:    bx lr
 entry:
   %add.ptr = getelementptr inbounds i8, i8* %p, i32 -255
   %0 = load i8, i8* %add.ptr, align 1
   %conv = zext i8 %0 to i32
   ret i32 %conv
 }
 
 define i32 @ldrh_ri_negative255(i8* %p) {
 ; CHECK-T1-LABEL: ldrh_ri_negative255:
 ; CHECK-T1:       @ %bb.0: @ %entry
-; CHECK-T1-NEXT:    movs r1, #254
-; CHECK-T1-NEXT:    mvns r1, r1
-; CHECK-T1-NEXT:    ldrh r0, [r0, r1]
+; CHECK-T1-NEXT:    subs r0, #255
+; CHECK-T1-NEXT:    ldrh r0, [r0]
 ; CHECK-T1-NEXT:    bx lr
 ;
 ; CHECK-T2-LABEL: ldrh_ri_negative255:
 ; CHECK-T2:       @ %bb.0: @ %entry
 ; CHECK-T2-NEXT:    ldrh r0, [r0, #-255]
 ; CHECK-T2-NEXT:    bx lr
 entry:
   %add.ptr = getelementptr inbounds i8, i8* %p, i32 -255
   %0 = bitcast i8* %add.ptr to i16*
   %1 = load i16, i16* %0, align 2
   %conv = zext i16 %1 to i32
   ret i32 %conv
 }
 
 define i32 @ldr_ri_negative255(i8* %p) {
 ; CHECK-T1-LABEL: ldr_ri_negative255:
 ; CHECK-T1:       @ %bb.0: @ %entry
-; CHECK-T1-NEXT:    movs r1, #254
-; CHECK-T1-NEXT:    mvns r1, r1
-; CHECK-T1-NEXT:    ldr r0, [r0, r1]
+; CHECK-T1-NEXT:    subs r0, #255
+; CHECK-T1-NEXT:    ldr r0, [r0]
 ; CHECK-T1-NEXT:    bx lr
 ;
 ; CHECK-T2-LABEL: ldr_ri_negative255:
 ; CHECK-T2:       @ %bb.0: @ %entry
 ; CHECK-T2-NEXT:    ldr r0, [r0, #-255]
 ; CHECK-T2-NEXT:    bx lr
 entry:
   %add.ptr = getelementptr inbounds i8, i8* %p, i32 -255
   %0 = bitcast i8* %add.ptr to i32*
   %1 = load i32, i32* %0, align 4
   ret i32 %1
 }
 
 define void @strb_ri_negative255(i8* %p, i32 %x) {
 ; CHECK-T1-LABEL: strb_ri_negative255:
 ; CHECK-T1:       @ %bb.0: @ %entry
-; CHECK-T1-NEXT:    movs r2, #254
-; CHECK-T1-NEXT:    mvns r2, r2
-; CHECK-T1-NEXT:    strb r1, [r0, r2]
+; CHECK-T1-NEXT:    subs r0, #255
+; CHECK-T1-NEXT:    strb r1, [r0]
 ; CHECK-T1-NEXT:    bx lr
 ;
 ; CHECK-T2-LABEL: strb_ri_negative255:
 ; CHECK-T2:       @ %bb.0: @ %entry
 ; CHECK-T2-NEXT:    strb r1, [r0, #-255]
 ; CHECK-T2-NEXT:    bx lr
 entry:
   %conv = trunc i32 %x to i8
   %add.ptr = getelementptr inbounds i8, i8* %p, i32 -255
   store i8 %conv, i8* %add.ptr, align 1
   ret void
 }
 
 define void @strh_ri_negative255(i8* %p, i32 %x) {
 ; CHECK-T1-LABEL: strh_ri_negative255:
 ; CHECK-T1:       @ %bb.0: @ %entry
-; CHECK-T1-NEXT:    movs r2, #254
-; CHECK-T1-NEXT:    mvns r2, r2
-; CHECK-T1-NEXT:    strh r1, [r0, r2]
+; CHECK-T1-NEXT:    subs r0, #255
+; CHECK-T1-NEXT:    strh r1, [r0]
 ; CHECK-T1-NEXT:    bx lr
 ;
 ; CHECK-T2-LABEL: strh_ri_negative255:
 ; CHECK-T2:       @ %bb.0: @ %entry
 ; CHECK-T2-NEXT:    strh r1, [r0, #-255]
 ; CHECK-T2-NEXT:    bx lr
 entry:
   %conv = trunc i32 %x to i16
   %add.ptr = getelementptr inbounds i8, i8* %p, i32 -255
   %0 = bitcast i8* %add.ptr to i16*
   store i16 %conv, i16* %0, align 2
   ret void
 }
 
 define void @str_ri_negative255(i8* %p, i32 %x) {
 ; CHECK-T1-LABEL: str_ri_negative255:
 ; CHECK-T1:       @ %bb.0: @ %entry
-; CHECK-T1-NEXT:    movs r2, #254
-; CHECK-T1-NEXT:    mvns r2, r2
-; CHECK-T1-NEXT:    str r1, [r0, r2]
+; CHECK-T1-NEXT:    subs r0, #255
+; CHECK-T1-NEXT:    str r1, [r0]
 ; CHECK-T1-NEXT:    bx lr
 ;
 ; CHECK-T2-LABEL: str_ri_negative255:
 ; CHECK-T2:       @ %bb.0: @ %entry
 ; CHECK-T2-NEXT:    str r1, [r0, #-255]
 ; CHECK-T2-NEXT:    bx lr
 entry:
   %add.ptr = getelementptr inbounds i8, i8* %p, i32 -255