[SystemZ] Don't reuse a sub that can overflow during isel.

[jonpa]

Summary

Don't use a possibly overflowing ISD::SUB node as a check against 0.

Removing this optimization for the cases of SUBs with the NSW/NUW flag set only gives only a negligible difference over benchmarks: ~50 more compares and no noticeable performance changes on SPEC. Removing the adjustForSubtraction() entirely however gave more changes: 1530 more compares, so it seems advisable to keep that function for the SUBs without NSW/NUW flags.

Fixes: https://github.com/llvm/llvm-project/issues/61268

[nikic]

llvm/lib/Target/SystemZ/SystemZISelLowering.cpp

2426

It would be better to drop the flags instead -- the general preference is to drop nowrap flags rather than block optimizations due to their presence.

[jonpa]

llvm/lib/Target/SystemZ/SystemZISelLowering.cpp

2426

Hmm - I wish I had a better grasp of this.. How is it that we can just drop the nowrap flag and let the program behavior change and print the wrong result?

[nikic]

llvm/lib/Target/SystemZ/SystemZISelLowering.cpp

2426

Dropping nowrap flag just means that instead of returning poison, the instruction will now return a well-defined value instead. This is always safe to do and will not change program behavior in the usual sense (this is called "refinement").

[jonpa]

ok, I think I got it now: we can drop the NW flags because the backend will then later properly handle OF as well during comparison elimination (and the program prints the right result).

Patch updated to drop the flags instead, per review.

[nikic]

LGTM

[jonpa]

In D145811#4192439, @nikic wrote:

LGTM

Thanks for review! @uweigand LGTY?

[uweigand]

LGTM as well, thanks!

llvm/lib/Target/SystemZ/SystemZISelLowering.cpp

 // or Y - X is also computed.  In that case it's better to compare the
 // result of the subtraction against zero.
 static void adjustForSubtraction(SelectionDAG &DAG, const SDLoc &DL,
                                  Comparison &C) {
   if (C.CCMask == SystemZ::CCMASK_CMP_EQ ||
       C.CCMask == SystemZ::CCMASK_CMP_NE) {
     for (SDNode *N : C.Op0->uses()) {
       if (N->getOpcode() == ISD::SUB &&
           ((N->getOperand(0) == C.Op0 && N->getOperand(1) == C.Op1) ||
            (N->getOperand(0) == C.Op1 && N->getOperand(1) == C.Op0))) {
+        // Disable the nsw and nuw flags: the backend needs to handle
+        // overflow as well during comparison elimination.
+        SDNodeFlags Flags = N->getFlags();
+        Flags.setNoSignedWrap(false);
+        Flags.setNoUnsignedWrap(false);
+        N->setFlags(Flags);
         C.Op0 = SDValue(N, 0);
         C.Op1 = DAG.getConstant(0, DL, N->getValueType(0));
         return;
       }
     }
   }
 }
 

llvm/test/CodeGen/SystemZ/int-cmp-62.ll

+; RUN: llc < %s -mtriple=s390x-linux-gnu -mcpu=z15 | FileCheck %s
+;
+; Test that a CC result of a sub that can overflow is tested with the right predicate.
+
+define i32 @fun0(i32 %a, i32 %b, ptr %dest) {
+; CHECK-LABEL: fun0
+; CHECK: s %r2, 0(%r4)
+; CHECK: bner %r14
+entry:
+  %cur = load i32, ptr %dest
+  %res = sub nsw i32 %a, %cur
+  %cmp = icmp ne i32 %a, %cur
+  br i1 %cmp, label %exit, label %store
+
+store:
+  store i32 %b, ptr %dest
+  br label %exit
+
+exit:
+  ret i32 %res
+}
+
+define i32 @fun1(i32 %a, i32 %b, ptr %dest) {
+; CHECK-LABEL: fun1
+; CHECK: s %r2, 0(%r4)
+; CHECK: bner %r14
+entry:
+  %cur = load i32, ptr %dest
+  %res = sub nuw i32 %a, %cur
+  %cmp = icmp ne i32 %a, %cur
+  br i1 %cmp, label %exit, label %store
+
+store:
+  store i32 %b, ptr %dest
+  br label %exit
+
+exit:
+  ret i32 %res
+}