[PATCH] D28175: [GlobalISel] Fix AArch64 ICMP instruction selection

[Kristof Beyls via Phabricator via llvm-commits]

kristof.beyls created this revision.
kristof.beyls added reviewers: t.p.northover, ab, qcolombet, dsanders, rovka.
kristof.beyls added a subscriber: llvm-commits.
Herald added subscribers: dberris, rengolin, aemerson.

CSINC increments the result by one when the condition is false. The existing code seems to assume it increases the result by one if the condition is true.
The simple fix is to inverse the predicate during ICMP instruction selection.

This dramatically increases the execution pass rate in the test-suite run with -mllvm -global-isel=true -mllvm -global-isel-abort=1 from 13.7% to 47.2%!
Not too surprising giving we were always lowering ICMP incorrectly so far...


https://reviews.llvm.org/D28175

Files:
  lib/Target/AArch64/AArch64InstructionSelector.cpp
  test/CodeGen/AArch64/GlobalISel/arm64-instructionselect.mir


Index: test/CodeGen/AArch64/GlobalISel/arm64-instructionselect.mir
===================================================================
--- test/CodeGen/AArch64/GlobalISel/arm64-instructionselect.mir
+++ test/CodeGen/AArch64/GlobalISel/arm64-instructionselect.mir
@@ -2858,13 +2858,13 @@
 
 # CHECK:  body:
 # CHECK:    %wzr = SUBSWrr %0, %0, implicit-def %nzcv
-# CHECK:    %1 = CSINCWr %wzr, %wzr, 0, implicit %nzcv
+# CHECK:    %1 = CSINCWr %wzr, %wzr, 1, implicit %nzcv
 
 # CHECK:    %xzr = SUBSXrr %2, %2, implicit-def %nzcv
-# CHECK:    %3 = CSINCWr %wzr, %wzr, 2, implicit %nzcv
+# CHECK:    %3 = CSINCWr %wzr, %wzr, 3, implicit %nzcv
 
 # CHECK:    %xzr = SUBSXrr %4, %4, implicit-def %nzcv
-# CHECK:    %5 = CSINCWr %wzr, %wzr, 1, implicit %nzcv
+# CHECK:    %5 = CSINCWr %wzr, %wzr, 0, implicit %nzcv
 
 body:             |
   bb.0:
Index: lib/Target/AArch64/AArch64InstructionSelector.cpp
===================================================================
--- lib/Target/AArch64/AArch64InstructionSelector.cpp
+++ lib/Target/AArch64/AArch64InstructionSelector.cpp
@@ -1076,8 +1076,9 @@
       return false;
     }
 
-    const AArch64CC::CondCode CC = changeICMPPredToAArch64CC(
-        (CmpInst::Predicate)I.getOperand(1).getPredicate());
+    const AArch64CC::CondCode invCC = changeICMPPredToAArch64CC(
+      CmpInst::getInversePredicate(
+        (CmpInst::Predicate)I.getOperand(1).getPredicate()));
 
     MachineInstr &CmpMI = *BuildMI(MBB, I, I.getDebugLoc(), TII.get(CmpOpc))
                                .addDef(ZReg)
@@ -1089,7 +1090,7 @@
              .addDef(I.getOperand(0).getReg())
              .addUse(AArch64::WZR)
              .addUse(AArch64::WZR)
-             .addImm(CC);
+             .addImm(invCC);
 
     constrainSelectedInstRegOperands(CmpMI, TII, TRI, RBI);
     constrainSelectedInstRegOperands(CSetMI, TII, TRI, RBI);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D28175.82727.patch
Type: text/x-patch
Size: 1869 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161230/d5026d9d/attachment.bin>