Skip to content

Commit c817509

Browse files
author
Thorsten Schütt
committed
[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
1 parent 9bccf61 commit c817509

File tree

2 files changed

+25
-6
lines changed

2 files changed

+25
-6
lines changed

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7830,9 +7830,7 @@ bool CombinerHelper::matchSuboCarryOut(const MachineInstr &MI,
78307830
case ConstantRange::OverflowResult::AlwaysOverflowsHigh: {
78317831
MatchInfo = [=](MachineIRBuilder &B) {
78327832
B.buildSub(Dst, LHS, RHS);
7833-
B.buildConstant(Carry, getICmpTrueVal(getTargetLowering(),
7834-
/*isVector=*/CarryTy.isVector(),
7835-
/*isFP=*/false));
7833+
B.buildConstant(Carry, 1);
78367834
};
78377835
return true;
78387836
}
@@ -7855,9 +7853,7 @@ bool CombinerHelper::matchSuboCarryOut(const MachineInstr &MI,
78557853
case ConstantRange::OverflowResult::AlwaysOverflowsHigh: {
78567854
MatchInfo = [=](MachineIRBuilder &B) {
78577855
B.buildSub(Dst, LHS, RHS);
7858-
B.buildConstant(Carry, getICmpTrueVal(getTargetLowering(),
7859-
/*isVector=*/CarryTy.isVector(),
7860-
/*isFP=*/false));
7856+
B.buildConstant(Carry, 1);
78617857
};
78627858
return true;
78637859
}

llvm/test/CodeGen/AArch64/GlobalISel/combine-overflow.mir

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,3 +277,26 @@ body: |
277277
$q1 = COPY %o_wide
278278
RET_ReallyLR implicit $w0
279279
...
280+
---
281+
name: usub_may_carry_non_const
282+
body: |
283+
bb.0:
284+
liveins: $w0, $w1
285+
; CHECK-LABEL: name: usub_may_carry_non_const
286+
; CHECK: liveins: $w0, $w1
287+
; CHECK-NEXT: {{ $}}
288+
; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s32) = COPY $w0
289+
; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s32) = COPY $w0
290+
; CHECK-NEXT: %sub:_(s32), %o:_(s1) = G_USUBO [[COPY]], [[COPY1]]
291+
; CHECK-NEXT: %o_wide:_(s32) = G_ZEXT %o(s1)
292+
; CHECK-NEXT: $w0 = COPY %sub(s32)
293+
; CHECK-NEXT: $w1 = COPY %o_wide(s32)
294+
; CHECK-NEXT: RET_ReallyLR implicit $w0
295+
%0:_(s32) = COPY $w0
296+
%1:_(s32) = COPY $w0
297+
%sub:_(s32), %o:_(s1) = G_USUBO %0, %1
298+
%o_wide:_(s32) = G_ZEXT %o(s1)
299+
$w0 = COPY %sub(s32)
300+
$w1 = COPY %o_wide
301+
RET_ReallyLR implicit $w0
302+
...

0 commit comments

Comments
 (0)