[PATCH] D90156: [PowerPC] Do not fold `cmp(d|w)` and `subf` instruction to `subf.` if `nsw` is not present

Kai Luo via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 26 07:15:47 PDT 2020


lkail created this revision.
lkail added reviewers: PowerPC, nemanjai.
Herald added subscribers: llvm-commits, shchenz, kbarton, hiraditya.
Herald added a project: LLVM.
lkail requested review of this revision.

In `PPCInstrInfo::optimizeCompareInstr` we seek opportunities to fold `cmp(d|w)` and `subf` as an `subf.`. However, if `subf.` gets overflow, `cr0` can't reflect the correct order, violating the semantics of `cmp(d|w)`.

Fixed https://bugs.llvm.org/show_bug.cgi?id=47830.

The test regression showed in `optcmp.ll` is due to another issue, I have posted another patch https://reviews.llvm.org/D89938 to fix.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90156

Files:
  llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
  llvm/test/CodeGen/PowerPC/optcmp.ll
  llvm/test/CodeGen/PowerPC/pr47830.ll


Index: llvm/test/CodeGen/PowerPC/pr47830.ll
===================================================================
--- llvm/test/CodeGen/PowerPC/pr47830.ll
+++ llvm/test/CodeGen/PowerPC/pr47830.ll
@@ -5,8 +5,9 @@
 define i64 @f(i64 %a, i64 %b) {
 ; CHECK-LABEL: f:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    sub. r3, r3, r4
-; CHECK-NEXT:    isellt r3, 0, r3
+; CHECK-NEXT:    sub r5, r3, r4
+; CHECK-NEXT:    cmpd r3, r4
+; CHECK-NEXT:    isellt r3, 0, r5
 ; CHECK-NEXT:    blr
   %c = icmp slt i64 %a, %b
   %d = sub i64 %a, %b
Index: llvm/test/CodeGen/PowerPC/optcmp.ll
===================================================================
--- llvm/test/CodeGen/PowerPC/optcmp.ll
+++ llvm/test/CodeGen/PowerPC/optcmp.ll
@@ -122,14 +122,16 @@
 define i64 @foolc(i64 %a, i64 %b, i64* nocapture %c) #0 {
 ; CHECK-LABEL: foolc:
 ; CHECK:       # %bb.0: # %entry
-; CHECK-NEXT:    sub. 6, 4, 3
+; CHECK-NEXT:    cmpd 4, 3
+; CHECK-NEXT:    sub 6, 4, 3
 ; CHECK-NEXT:    isellt 3, 3, 4
 ; CHECK-NEXT:    std 6, 0(5)
 ; CHECK-NEXT:    blr
 ;
 ; CHECK-NO-ISEL-LABEL: foolc:
 ; CHECK-NO-ISEL:       # %bb.0: # %entry
-; CHECK-NO-ISEL-NEXT:    sub. 6, 4, 3
+; CHECK-NO-ISEL-NEXT:    cmpd 4, 3
+; CHECK-NO-ISEL-NEXT:    sub 6, 4, 3
 ; CHECK-NO-ISEL-NEXT:    bc 12, 0, .LBB4_2
 ; CHECK-NO-ISEL-NEXT:  # %bb.1: # %entry
 ; CHECK-NO-ISEL-NEXT:    ori 3, 4, 0
@@ -148,14 +150,16 @@
 define i64 @foold(i64 %a, i64 %b, i64* nocapture %c) #0 {
 ; CHECK-LABEL: foold:
 ; CHECK:       # %bb.0: # %entry
-; CHECK-NEXT:    sub. 6, 4, 3
+; CHECK-NEXT:    cmpd 4, 3
+; CHECK-NEXT:    sub 6, 4, 3
 ; CHECK-NEXT:    iselgt 3, 3, 4
 ; CHECK-NEXT:    std 6, 0(5)
 ; CHECK-NEXT:    blr
 ;
 ; CHECK-NO-ISEL-LABEL: foold:
 ; CHECK-NO-ISEL:       # %bb.0: # %entry
-; CHECK-NO-ISEL-NEXT:    sub. 6, 4, 3
+; CHECK-NO-ISEL-NEXT:    cmpd 4, 3
+; CHECK-NO-ISEL-NEXT:    sub 6, 4, 3
 ; CHECK-NO-ISEL-NEXT:    bc 12, 1, .LBB5_2
 ; CHECK-NO-ISEL-NEXT:  # %bb.1: # %entry
 ; CHECK-NO-ISEL-NEXT:    ori 3, 4, 0
Index: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
===================================================================
--- llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
+++ llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
@@ -2090,6 +2090,12 @@
   if (NewOpC == -1)
     return false;
 
+  // This transformation should not be performed if `nsw` is missing. Since if
+  // there is overflow, CR bits do not reflect correct order.
+  if (!equalityOnly && (NewOpC == PPC::SUBF_rec || NewOpC == PPC::SUBF8_rec) &&
+      Sub && !Sub->getFlag(MachineInstr::NoSWrap))
+    return false;
+
   // If we have SUB(r1, r2) and CMP(r2, r1), the condition code based on CMP
   // needs to be updated to be based on SUB.  Push the condition code
   // operands to OperandsToUpdate.  If it is safe to remove CmpInstr, the


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D90156.300657.patch
Type: text/x-patch
Size: 2760 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201026/d0c92698/attachment.bin>


More information about the llvm-commits mailing list