[ARM] Mark -1 as cheap in xor's for thumb1

[dmgreen]

Summary

We can always convert xor %a, -1 into MVN, even in thumb 1 where the -1
would not otherwise be considered a cheap constant. This prevents the
-1's from being pulled out into constants and potentially hoisted.

The loop_2 tests are the only ones that change here.

[spatel]

This is the same motivating case as D42951 - abandon that patch?

I haven't looked at ConstantHoisting much, but this seems like a good solution to the problem. I wonder if we could generalize this for all targets by checking if a 'not' op is available.

Might want to commit the tests first with auto-generated output using utils/update_llc_test_checks.py, so we just see the improvement here.

There's a FIXME note about global isel for the div-by-constant case, so this problem also disappears once we have that? Maybe the whole thing disappears, so it's not worth repeating the comment.

[dmgreen]

In D43451#1012076, @spatel wrote:

This is the same motivating case as D42951 - abandon that patch?

Yep. This is part one out of two or three. I'm not sure yet, depends on how part 2 looks. I don't think it will solve the register pressure part of the problem (not entirely at least), but it should always be a net gain when it comes up.

I haven't looked at ConstantHoisting much, but this seems like a good solution to the problem. I wonder if we could generalize this for all targets by checking if a 'not' op is available.

Hmm. I didn't even consider other targets. I think the problem is getIntImmCost above (without an opcode) returning 2 for thumb1 targets. Everything else (Thumb2/Arm) is going to treat -1 as a cheap constant anyway. For other targets, I believe the default is to return Free for all constants.

Might want to commit the tests first with auto-generated output using utils/update_llc_test_checks.py, so we just see the improvement here.

Sure, sounds sensible. Like I said it's just loop_2 that's updated here.

There's a FIXME note about global isel for the div-by-constant case, so this problem also disappears once we have that? Maybe the whole thing disappears, so it's not worth repeating the comment.

Yeah I think so. I presume global ISel will have a much better time looking across basic blocks and a lot of codegen prepare/constant merge wont be needed any more. I think that comment applies to all costs in this function.

[dmgreen]

I realised I could just show the updates here without having to commit the changes first. So that's what this is.

I've not committed the left hand side yet, let me know if I should anyway. Otherwise this should show the differences.

[spatel]

LGTM.

[efriedma]

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

131

getSExtValue() will crash if Imm doesn't fit into an int64_t. Maybe use isAllOnesValue()?

[dmgreen]

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

131

Ah. Where it's not -1. Of course. Sorry about that, thanks for the catch.

https://reviews.llvm.org/D43549

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

if (Opcode == Instruction::ICmp && Imm.isNegative() &&
     if (ST->isThumb2() && NegImm < 1<<12)
       // icmp X, #-C -> cmn X, #C
       return 0;
     if (ST->isThumb() && NegImm < 1<<8)
       // icmp X, #-C -> adds X, #C
       return 0;
   }
 
+  // xor a, -1 can always be folded to MVN
+  if (Opcode == Instruction::Xor) {
+    int64_t NegImm = Imm.getSExtValue();
+    if (NegImm == -1)
+      return 0;
+  }
+
   return getIntImmCost(Imm, Ty);
 }
 
 int ARMTTIImpl::getCastInstrCost(unsigned Opcode, Type *Dst, Type *Src,
                                  const Instruction *I) {
   int ISD = TLI->InstructionOpcodeToISD(Opcode);
   assert(ISD && "Invalid opcode");
 

llvm/trunk/test/CodeGen/Thumb/mvn.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=thumbv6m-eabi -asm-verbose=false %s -o - | FileCheck %s
+
+define void @test8(i8* %a) {
+; CHECK-LABEL: test8:
+; CHECK:         ldrb r1, [r0]
+; CHECK-NEXT:    mvns r1, r1
+; CHECK-NEXT:    strb r1, [r0]
+; CHECK-NEXT:    bx lr
+  %x = load i8, i8* %a
+  %xn = xor i8 %x, -1
+  store i8 %xn, i8* %a
+  ret void
+}
+
+define void @test8_2(i8* %a, i8* %b) {
+; CHECK-LABEL: test8_2:
+; CHECK:         ldrb r2, [r1]
+; CHECK-NEXT:    ldrb r3, [r0]
+; CHECK-NEXT:    mvns r3, r3
+; CHECK-NEXT:    strb r3, [r0]
+; CHECK-NEXT:    mvns r0, r2
+; CHECK-NEXT:    strb r0, [r1]
+; CHECK-NEXT:    bx lr
+  %x = load i8, i8* %a
+  %y = load i8, i8* %b
+  %xn = xor i8 %x, -1
+  %yn = xor i8 %y, -1
+  store i8 %xn, i8* %a
+  store i8 %yn, i8* %b
+  ret void
+}
+
+define void @loop8(i8* %a) {
+; CHECK-LABEL: loop8:
+; CHECK:         movs r1, #0
+; CHECK-NEXT:  .LBB2_1:
+; CHECK-NEXT:    ldrb r2, [r0, r1]
+; CHECK-NEXT:    mvns r2, r2
+; CHECK-NEXT:    strb r2, [r0, r1]
+; CHECK-NEXT:    adds r1, r1, #1
+; CHECK-NEXT:    cmp r1, #10
+; CHECK-NEXT:    bne .LBB2_1
+; CHECK-NEXT:    bx lr
+entry:
+  br label %for.body
+
+for.body:
+  %i = phi i32 [ 0, %entry ], [ %inc, %for.body ]
+  %arrayidx = getelementptr inbounds i8, i8* %a, i32 %i
+  %x = load i8, i8* %arrayidx
+  %xn = xor i8 %x, -1
+  store i8 %xn, i8* %arrayidx
+  %inc = add nuw nsw i32 %i, 1
+  %exitcond = icmp eq i32 %inc, 10
+  br i1 %exitcond, label %for.cond.cleanup, label %for.body
+
+for.cond.cleanup:
+  ret void
+}
+
+define void @loop8_2(i8* %a, i8* %b) {
+; CHECK-LABEL: loop8_2:
+; CHECK:         .save {r4, lr}
+; CHECK-NEXT:    push {r4, lr}
+; CHECK-NEXT:    movs r2, #0
+; CHECK-NEXT:  .LBB3_1:
+; CHECK-NEXT:    ldrb r3, [r1, r2]
+; CHECK-NEXT:    ldrb r4, [r0, r2]
+; CHECK-NEXT:    mvns r4, r4
+; CHECK-NEXT:    strb r4, [r0, r2]
+; CHECK-NEXT:    mvns r3, r3
+; CHECK-NEXT:    strb r3, [r1, r2]
+; CHECK-NEXT:    adds r2, r2, #1
+; CHECK-NEXT:    cmp r2, #10
+; CHECK-NEXT:    bne .LBB3_1
+; CHECK-NEXT:    pop {r4, pc}
+entry:
+  br label %for.body
+
+for.body:
+  %i = phi i32 [ 0, %entry ], [ %inc, %for.body ]
+  %arrayidx = getelementptr inbounds i8, i8* %a, i32 %i
+  %arrayidx2 = getelementptr inbounds i8, i8* %b, i32 %i
+  %x = load i8, i8* %arrayidx
+  %y = load i8, i8* %arrayidx2
+  %xn = xor i8 %x, -1
+  %yn = xor i8 %y, -1
+  store i8 %xn, i8* %arrayidx
+  store i8 %yn, i8* %arrayidx2
+  %inc = add nuw nsw i32 %i, 1
+  %exitcond = icmp eq i32 %inc, 10
+  br i1 %exitcond, label %for.cond.cleanup, label %for.body
+
+for.cond.cleanup:
+  ret void
+}
+
+define void @test32(i32* %a) {
+; CHECK-LABEL: test32:
+; CHECK:         ldr r1, [r0]
+; CHECK-NEXT:    mvns r1, r1
+; CHECK-NEXT:    str r1, [r0]
+; CHECK-NEXT:    bx lr
+  %x = load i32, i32* %a
+  %xn = xor i32 %x, -1
+  store i32 %xn, i32* %a
+  ret void
+}
+
+define void @test32_2(i32* %a, i32* %b) {
+; CHECK-LABEL: test32_2:
+; CHECK:         ldr r2, [r1]
+; CHECK-NEXT:    ldr r3, [r0]
+; CHECK-NEXT:    mvns r3, r3
+; CHECK-NEXT:    str r3, [r0]
+; CHECK-NEXT:    mvns r0, r2
+; CHECK-NEXT:    str r0, [r1]
+; CHECK-NEXT:    bx lr
+  %x = load i32, i32* %a
+  %y = load i32, i32* %b
+  %xn = xor i32 %x, -1
+  %yn = xor i32 %y, -1
+  store i32 %xn, i32* %a
+  store i32 %yn, i32* %b
+  ret void
+}
+
+define void @loop32(i32* %a) {
+; CHECK-LABEL: loop32:
+; CHECK:         movs r1, #0
+; CHECK-NEXT:  .LBB6_1:
+; CHECK-NEXT:    ldr r2, [r0, r1]
+; CHECK-NEXT:    mvns r2, r2
+; CHECK-NEXT:    str r2, [r0, r1]
+; CHECK-NEXT:    adds r1, r1, #4
+; CHECK-NEXT:    cmp r1, #40
+; CHECK-NEXT:    bne .LBB6_1
+; CHECK-NEXT:    bx lr
+entry:
+  br label %for.body
+
+for.body:
+  %i = phi i32 [ 0, %entry ], [ %inc, %for.body ]
+  %arrayidx = getelementptr inbounds i32, i32* %a, i32 %i
+  %x = load i32, i32* %arrayidx
+  %xn = xor i32 %x, -1
+  store i32 %xn, i32* %arrayidx
+  %inc = add nuw nsw i32 %i, 1
+  %exitcond = icmp eq i32 %inc, 10
+  br i1 %exitcond, label %for.cond.cleanup, label %for.body
+
+for.cond.cleanup:
+  ret void
+}
+
+define void @loop32_2(i32* %a, i32* %b) {
+; CHECK-LABEL: loop32_2:
+; CHECK:         .save {r4, lr}
+; CHECK-NEXT:    push {r4, lr}
+; CHECK-NEXT:    movs r2, #0
+; CHECK-NEXT:  .LBB7_1:
+; CHECK-NEXT:    ldr r3, [r1, r2]
+; CHECK-NEXT:    ldr r4, [r0, r2]
+; CHECK-NEXT:    mvns r4, r4
+; CHECK-NEXT:    str r4, [r0, r2]
+; CHECK-NEXT:    mvns r3, r3
+; CHECK-NEXT:    str r3, [r1, r2]
+; CHECK-NEXT:    adds r2, r2, #4
+; CHECK-NEXT:    cmp r2, #40
+; CHECK-NEXT:    bne .LBB7_1
+; CHECK-NEXT:    pop {r4, pc}
+entry:
+  br label %for.body
+
+for.body:
+  %i = phi i32 [ 0, %entry ], [ %inc, %for.body ]
+  %arrayidx = getelementptr inbounds i32, i32* %a, i32 %i
+  %arrayidx2 = getelementptr inbounds i32, i32* %b, i32 %i
+  %x = load i32, i32* %arrayidx
+  %y = load i32, i32* %arrayidx2
+  %xn = xor i32 %x, -1
+  %yn = xor i32 %y, -1
+  store i32 %xn, i32* %arrayidx
+  store i32 %yn, i32* %arrayidx2
+  %inc = add nuw nsw i32 %i, 1
+  %exitcond = icmp eq i32 %inc, 10
+  br i1 %exitcond, label %for.cond.cleanup, label %for.body
+
+for.cond.cleanup:
+  ret void
+}
+