[llvm] [GlobalISel] Fix [SU]SUBO combine (PR #116998)

[Thorsten Schütt via llvm-commits]

https://github.com/tschuett created https://github.com/llvm/llvm-project/pull/116998

The carry-out can participate in several positions:
* the condition of a select
* the condition of a conditional branch
* the carry-in of addes and subes (narrowScalarAddSub)

ADCS: Add with carry and set flags

Add with Carry, setting flags, adds two register values and the Carry flag value, and writes the result to the destination register. It updates the condition flags based on the result.

Note that the carry-in participates in the addition. It must be a 0 or 1 value. The carry-in/out values must be 0 or 1 for the addos, subos, addes, and subos.

Including, but limited to the lowering for G_USUBO is buggy: `MIRBuilder.buildICmp(CmpInst::ICMP_ULT, BorrowOut, LHS, RHS);`

The carry-out is target dependent and can lead to surprising and non-deterministic results when used as carry-in.

narrowScalarAddSub can build chains of addes/subes. When the ops are not negal for the current target and get lowered. They become chains of adds/subs and icmps. The result will be come target dependent, which is incorrect.

Lowering should write selects between one and zero constants into carry-out registers.

AARrch64: ZeroOrOneBooleanContent
AMDGPU: ZeroOrNegativeOneBooleanContent
 and ZeroOrOneBooleanContent
RISCV: ZeroOrOneBooleanContent

>From c8175093896ff9988043e93cbfa397bc8d15aded Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thorsten=20Sch=C3=BCtt?= <schuett at gmail.com>
Date: Tue, 19 Nov 2024 21:03:18 +0100
Subject: [PATCH] [GlobalISel] Fix [SU]SUBO combine

The carry-out can participate in several positions:
* the condition of a select
* the condition of a conditional branch
* the carry-in of addes and subes (narrowScalarAddSub)

ADCS: Add with carry and set flags

Add with Carry, setting flags, adds two register values and the Carry
flag value, and writes the result to the destination register. It
updates the condition flags based on the result.

Note that the carry-in participates in the addition. It must be a 0 or
1 value. The carry-in/out values must be 0 or 1 for the addos, subos,
addes, and subos.

Including, but limited to the lowering for G_USUBO is buggy:
`MIRBuilder.buildICmp(CmpInst::ICMP_ULT, BorrowOut, LHS, RHS);`

The carry-out is target dependent and can lead to surprising and
non-deterministic results when used as carry-in.

narrowScalarAddSub can build chains of addes/subes. When the ops are
not negal for the current target and get lowered. They become chains
of adds/subs and icmps. The result will be come target dependent,
which is incorrect.

Lowering should write selects between one and zero constants into
carry-out registers.

AARrch64: ZeroOrOneBooleanContent
AMDGPU: ZeroOrNegativeOneBooleanContent
 and ZeroOrOneBooleanContent
RISCV: ZeroOrOneBooleanContent
---
 .../lib/CodeGen/GlobalISel/CombinerHelper.cpp |  8 ++-----
 .../AArch64/GlobalISel/combine-overflow.mir   | 23 +++++++++++++++++++
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
index d95fc8cfbcf558..027e4ee7159b41 100644
--- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
@@ -7830,9 +7830,7 @@ bool CombinerHelper::matchSuboCarryOut(const MachineInstr &MI,
     case ConstantRange::OverflowResult::AlwaysOverflowsHigh: {
       MatchInfo = [=](MachineIRBuilder &B) {
         B.buildSub(Dst, LHS, RHS);
-        B.buildConstant(Carry, getICmpTrueVal(getTargetLowering(),
-                                              /*isVector=*/CarryTy.isVector(),
-                                              /*isFP=*/false));
+        B.buildConstant(Carry, 1);
       };
       return true;
     }
@@ -7855,9 +7853,7 @@ bool CombinerHelper::matchSuboCarryOut(const MachineInstr &MI,
   case ConstantRange::OverflowResult::AlwaysOverflowsHigh: {
     MatchInfo = [=](MachineIRBuilder &B) {
       B.buildSub(Dst, LHS, RHS);
-      B.buildConstant(Carry, getICmpTrueVal(getTargetLowering(),
-                                            /*isVector=*/CarryTy.isVector(),
-                                            /*isFP=*/false));
+      B.buildConstant(Carry, 1);
     };
     return true;
   }
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/combine-overflow.mir b/llvm/test/CodeGen/AArch64/GlobalISel/combine-overflow.mir
index 20cba54923548e..8f24ee76f82b32 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/combine-overflow.mir
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/combine-overflow.mir
@@ -277,3 +277,26 @@ body:             |
     $q1 = COPY %o_wide
     RET_ReallyLR implicit $w0
 ...
+---
+name:            usub_may_carry_non_const
+body:             |
+  bb.0:
+    liveins: $w0, $w1
+    ; CHECK-LABEL: name: usub_may_carry_non_const
+    ; CHECK: liveins: $w0, $w1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s32) = COPY $w0
+    ; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s32) = COPY $w0
+    ; CHECK-NEXT: %sub:_(s32), %o:_(s1) = G_USUBO [[COPY]], [[COPY1]]
+    ; CHECK-NEXT: %o_wide:_(s32) = G_ZEXT %o(s1)
+    ; CHECK-NEXT: $w0 = COPY %sub(s32)
+    ; CHECK-NEXT: $w1 = COPY %o_wide(s32)
+    ; CHECK-NEXT: RET_ReallyLR implicit $w0
+    %0:_(s32) = COPY $w0
+    %1:_(s32) = COPY $w0
+    %sub:_(s32), %o:_(s1) = G_USUBO %0, %1
+    %o_wide:_(s32) = G_ZEXT %o(s1)
+    $w0 = COPY %sub(s32)
+    $w1 = COPY %o_wide
+    RET_ReallyLR implicit $w0
+...