[AArch64] AArch64CondBrTuningPass generates wrong branch instructions

[labrinea]

Summary

Some conditional branch instructions generated by this pass are checking the wrong condition code. The instructions TBZ and TBNZ are transformed into B.GE and B.LT instead of B.PL and B.MI respectively. They should only be checking the Negative bit.

[gberry]

Good catch. @mcrosier These test could probably be improved a little by making clear which way the branch was going by checking for the store instruction before the ret.

[gberry]

lib/Target/AArch64/AArch64CondBrTuning.cpp

25

This comment should be fixed.

[mcrosier]

Thanks for the fix, Alexandros!

I'll improve the test cases shortly, Geoff.

lib/Target/AArch64/AArch64CondBrTuning.cpp

25

I'll update this shortly.

lib/Target/AArch64/AArch64CondBrTuning.cpp

 ///
 /// 1) add w8, w0, w1  -> cmn w0, w1             ; CMN is an alias of ADDS.
 ///    cbz w8, .LBB_2  -> b.eq .LBB0_2
 ///
 /// 2) add w8, w0, w1  -> adds w8, w0, w1        ; w8 has multiple uses.
 ///    cbz w8, .LBB1_2 -> b.eq .LBB1_2
 ///
 /// 3) sub w8, w0, w1       -> subs w8, w0, w1   ; w8 has multiple uses.
 ///    tbz w8, #31, .LBB6_2 -> b.ge .LBB6_2
 ///
 //===----------------------------------------------------------------------===//
 
 #include "AArch64.h"
 #include "AArch64Subtarget.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
case AArch64::CBZX:
     CC = AArch64CC::EQ;
     break;
   case AArch64::CBNZW:
   case AArch64::CBNZX:
     CC = AArch64CC::NE;
     break;
   case AArch64::TBZW:
   case AArch64::TBZX:
-    CC = AArch64CC::GE;
+    CC = AArch64CC::PL;
     break;
   case AArch64::TBNZW:
   case AArch64::TBNZX:
-    CC = AArch64CC::LT;
+    CC = AArch64CC::MI;
     break;
   }
   return BuildMI(*MI.getParent(), MI, MI.getDebugLoc(), TII->get(AArch64::Bcc))
       .addImm(CC)
       .addMBB(TargetMBB);
 }
 
 bool AArch64CondBrTuning::tryToTuneBranch(MachineInstr &MI,

test/CodeGen/AArch64/cond-br-tuning.ll

L1:
   ret void
 L2:
   store i32 1, i32* %ptr, align 4
   ret void
 }
 
 ; CHECK-LABEL: test_add_tbz:
 ; CHECK: adds
-; CHECK: b.ge
+; CHECK: b.pl
 ; CHECK: ret
 define void @test_add_tbz(i32 %a, i32 %b, i32* %ptr) {
 entry:
   %add = add nsw i32 %a, %b
   %cmp36 = icmp sge i32 %add, 0
   br i1 %cmp36, label %L2, label %L1
 L1:
   store i32 %add, i32* %ptr, align 8
   br label %L2
 L2:
   ret void
 }
 
 ; CHECK-LABEL: test_subs_tbz:
 ; CHECK: subs
-; CHECK: b.ge
+; CHECK: b.pl
 ; CHECK: ret
 define void @test_subs_tbz(i32 %a, i32 %b, i32* %ptr) {
 entry:
   %sub = sub nsw i32 %a, %b
   %cmp36 = icmp sge i32 %sub, 0
   br i1 %cmp36, label %L2, label %L1
 L1:
   store i32 %sub, i32* %ptr, align 8
   br label %L2
 L2:
   ret void
 }
 
 ; CHECK-LABEL: test_add_tbnz
 ; CHECK: adds
-; CHECK: b.lt
+; CHECK: b.mi
 ; CHECK: ret
 define void @test_add_tbnz(i32 %a, i32 %b, i32* %ptr) {
 entry:
   %add = add nsw i32 %a, %b
   %cmp36 = icmp slt i32 %add, 0
   br i1 %cmp36, label %L2, label %L1
 L1:
   store i32 %add, i32* %ptr, align 8
   br label %L2
 L2:
   ret void
 }
 
 ; CHECK-LABEL: test_subs_tbnz
 ; CHECK: subs
-; CHECK: b.lt
+; CHECK: b.mi
 ; CHECK: ret
 define void @test_subs_tbnz(i32 %a, i32 %b, i32* %ptr) {
 entry:
   %sub = sub nsw i32 %a, %b
   %cmp36 = icmp slt i32 %sub, 0
   br i1 %cmp36, label %L2, label %L1
 L1:
   store i32 %sub, i32* %ptr, align 8