[RISCV][GlobalISel][TargetLowering] Change SSUBO to do (LHS < RHS) XOR (RESULT < 0)

[AZero13]

Taking inspiration from the way ARM detects overflow, we should check the LHS and RHS as well as the difference when detecting subtraction.

[llvmbot]

@llvm/pr-subscribers-backend-risc-v
@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-backend-amdgpu
@llvm/pr-subscribers-llvm-globalisel

@llvm/pr-subscribers-llvm-selectiondag

Author: AZero13 (AZero13)

Changes

This folds better.

---

Full diff: https://github.com/llvm/llvm-project/pull/150872.diff

1 Files Affected:

• (modified) llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp (+19-7)

diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
index 1764910861df4..cbd42251bc841 100644
--- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -11447,13 +11447,25 @@ void TargetLowering::expandSADDSUBO(
   // For a subtraction, the result should be less than one of the operands
   // (LHS) if and only if the other operand (RHS) is (non-zero) positive,
   // otherwise there will be overflow.
-  SDValue ResultLowerThanLHS = DAG.getSetCC(dl, OType, Result, LHS, ISD::SETLT);
-  SDValue ConditionRHS =
-      DAG.getSetCC(dl, OType, RHS, Zero, IsAdd ? ISD::SETLT : ISD::SETGT);
-
-  Overflow = DAG.getBoolExtOrTrunc(
-      DAG.getNode(ISD::XOR, dl, OType, ConditionRHS, ResultLowerThanLHS), dl,
-      ResultType, ResultType);
+  
+  if (IsAdd) {
+    // For addition, the result should be less than one of the operands (LHS)
+    // if and only if the other operand (RHS) is negative, otherwise there will
+    // be overflow.
+    SDValue ResultLowerThanLHS = DAG.getSetCC(dl, OType, Result, LHS, ISD::SETLT);
+    SDValue ConditionRHS = DAG.getSetCC(dl, OType, RHS, Zero, ISD::SETLT);
+    Overflow = DAG.getBoolExtOrTrunc(
+        DAG.getNode(ISD::XOR, dl, OType, ConditionRHS, ResultLowerThanLHS), dl,
+        ResultType, ResultType);
+  } else {
+    // For subtraction, overflow occurs when the signed comparison of operands
+    // doesn't match the sign of the result
+    SDValue LHSLessThanRHS = DAG.getSetCC(dl, OType, LHS, RHS, ISD::SETLT);
+    SDValue ResultNegative = DAG.getSetCC(dl, OType, Result, Zero, ISD::SETLT);
+    Overflow = DAG.getBoolExtOrTrunc(
+        DAG.getNode(ISD::XOR, dl, OType, LHSLessThanRHS, ResultNegative), dl,
+        ResultType, ResultType);
+  }
 }
 
 bool TargetLowering::expandMULO(SDNode *Node, SDValue &Result,

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[AZero13]

Have to rebase to resolve conflicts with ssubo.ll ...

[topperc]

The title needs to include GlobalISel and RISCV too since TargetLowering is a SelectionDAG file.

[RKSimon]

Please can you improve the patch summary description.

[topperc]

Title should mention SSUBO or sub overflow. Just saying "subtraction" is confusing.

[topperc]

The title needs to include GlobalISel and RISCV too since TargetLowering is a SelectionDAG file.

You changed the title for one of my comments but this is still not addressed