Skip to content

Commit 3c413ff

Browse files
tobias-stadlertobias-stadler
committed
[AArch64][GlobalISel] Fix miscompile on carry-in selection
Eliding the NZCV setting instruction for G_UADDE/... is illegal if it causes the instruction that defines the carry vReg to become dead. We fix this by recursively selecting this instruction before continuing to select the current instruction.
1 parent b8b7c3b commit 3c413ff

File tree

5 files changed

+143
-1
lines changed

5 files changed

+143
-1
lines changed

‎llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,11 @@ class AArch64InstructionSelector : public InstructionSelector {
102102
// An early selection function that runs before the selectImpl() call.
103103
bool earlySelect(MachineInstr &I);
104104

105+
/// Save state that is shared between select calls, call select on \p I and
106+
/// then restore the saved state. This can be used to recursively call select
107+
/// within a select call.
108+
bool selectAndRestoreState(MachineInstr &I);
109+
105110
// Do some preprocessing of G_PHIs before we begin selection.
106111
void processPHIs(MachineFunction &MF);
107112

@@ -3552,6 +3557,13 @@ bool AArch64InstructionSelector::select(MachineInstr &I) {
35523557
return false;
35533558
}
35543559

3560+
bool AArch64InstructionSelector::selectAndRestoreState(MachineInstr &I) {
3561+
MachineIRBuilderState OldMIBState = MIB.getState();
3562+
bool Success = select(I);
3563+
MIB.getState() = std::move(OldMIBState);
3564+
return Success;
3565+
}
3566+
35553567
bool AArch64InstructionSelector::selectReduction(MachineInstr &I,
35563568
MachineRegisterInfo &MRI) {
35573569
Register VecReg = I.getOperand(1).getReg();
@@ -4749,11 +4761,17 @@ MachineInstr *AArch64InstructionSelector::emitCarryIn(MachineInstr &I,
47494761
// emit a carry generating instruction. E.g. for G_UADDE/G_USUBE sequences
47504762
// generated during legalization of wide add/sub. This optimization depends on
47514763
// these sequences not being interrupted by other instructions.
4764+
// We have to select the previous instruction before the carry-using
4765+
// instruction is deleted by the calling function, otherwise the previous
4766+
// instruction might become dead and would get deleted.
47524767
MachineInstr *SrcMI = MRI->getVRegDef(CarryReg);
47534768
if (SrcMI == I.getPrevNode()) {
47544769
if (auto *CarrySrcMI = dyn_cast<GAddSubCarryOut>(SrcMI)) {
47554770
bool ProducesNegatedCarry = CarrySrcMI->isSub();
4756-
if (NeedsNegatedCarry == ProducesNegatedCarry && CarrySrcMI->isUnsigned())
4771+
if (NeedsNegatedCarry == ProducesNegatedCarry &&
4772+
CarrySrcMI->isUnsigned() &&
4773+
CarrySrcMI->getCarryOutReg() == CarryReg &&
4774+
selectAndRestoreState(*SrcMI))
47574775
return nullptr;
47584776
}
47594777
}

‎llvm/test/CodeGen/AArch64/GlobalISel/select-sadde.mir

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,3 +175,34 @@ body: |
175175
$x2 = COPY %9(s64)
176176
RET_ReallyLR implicit $x0, implicit $x1, implicit $x2
177177
...
178+
...
179+
---
180+
name: sadde_opt_prev_dead
181+
alignment: 4
182+
legalized: true
183+
regBankSelected: true
184+
tracksRegLiveness: true
185+
body: |
186+
bb.1:
187+
liveins: $x0, $x1, $x2, $x3
188+
; CHECK-LABEL: name: sadde_opt_prev_dead
189+
; CHECK: liveins: $x0, $x1, $x2, $x3
190+
; CHECK-NEXT: {{ $}}
191+
; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr64 = COPY $x0
192+
; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr64 = COPY $x1
193+
; CHECK-NEXT: [[COPY2:%[0-9]+]]:gpr64 = COPY $x2
194+
; CHECK-NEXT: [[COPY3:%[0-9]+]]:gpr64 = COPY $x3
195+
; CHECK-NEXT: [[ADDSXrr:%[0-9]+]]:gpr64 = ADDSXrr [[COPY]], [[COPY2]], implicit-def $nzcv
196+
; CHECK-NEXT: [[ADCSXr:%[0-9]+]]:gpr64 = ADCSXr [[COPY1]], [[COPY3]], implicit-def $nzcv, implicit $nzcv
197+
; CHECK-NEXT: [[CSINCWr:%[0-9]+]]:gpr32 = CSINCWr $wzr, $wzr, 7, implicit $nzcv
198+
; CHECK-NEXT: $w0 = COPY [[CSINCWr]]
199+
; CHECK-NEXT: RET_ReallyLR implicit $w0
200+
%0:gpr(s64) = COPY $x0
201+
%1:gpr(s64) = COPY $x1
202+
%2:gpr(s64) = COPY $x2
203+
%3:gpr(s64) = COPY $x3
204+
%4:gpr(s64), %5:gpr(s32) = G_UADDO %0, %2
205+
%6:gpr(s64), %7:gpr(s32) = G_SADDE %1, %3, %5
206+
$w0 = COPY %7(s32)
207+
RET_ReallyLR implicit $w0
208+
...

‎llvm/test/CodeGen/AArch64/GlobalISel/select-ssube.mir

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,3 +175,34 @@ body: |
175175
$x2 = COPY %9(s64)
176176
RET_ReallyLR implicit $x0, implicit $x1, implicit $x2
177177
...
178+
...
179+
---
180+
name: ssube_opt_prev_dead
181+
alignment: 4
182+
legalized: true
183+
regBankSelected: true
184+
tracksRegLiveness: true
185+
body: |
186+
bb.1:
187+
liveins: $x0, $x1, $x2, $x3
188+
; CHECK-LABEL: name: ssube_opt_prev_dead
189+
; CHECK: liveins: $x0, $x1, $x2, $x3
190+
; CHECK-NEXT: {{ $}}
191+
; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr64 = COPY $x0
192+
; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr64 = COPY $x1
193+
; CHECK-NEXT: [[COPY2:%[0-9]+]]:gpr64 = COPY $x2
194+
; CHECK-NEXT: [[COPY3:%[0-9]+]]:gpr64 = COPY $x3
195+
; CHECK-NEXT: [[SUBSXrr:%[0-9]+]]:gpr64 = SUBSXrr [[COPY]], [[COPY2]], implicit-def $nzcv
196+
; CHECK-NEXT: [[SBCSXr:%[0-9]+]]:gpr64 = SBCSXr [[COPY1]], [[COPY3]], implicit-def $nzcv, implicit $nzcv
197+
; CHECK-NEXT: [[CSINCWr:%[0-9]+]]:gpr32 = CSINCWr $wzr, $wzr, 7, implicit $nzcv
198+
; CHECK-NEXT: $w0 = COPY [[CSINCWr]]
199+
; CHECK-NEXT: RET_ReallyLR implicit $w0
200+
%0:gpr(s64) = COPY $x0
201+
%1:gpr(s64) = COPY $x1
202+
%2:gpr(s64) = COPY $x2
203+
%3:gpr(s64) = COPY $x3
204+
%4:gpr(s64), %5:gpr(s32) = G_USUBO %0, %2
205+
%6:gpr(s64), %7:gpr(s32) = G_SSUBE %1, %3, %5
206+
$w0 = COPY %7(s32)
207+
RET_ReallyLR implicit $w0
208+
...

‎llvm/test/CodeGen/AArch64/GlobalISel/select-uadde.mir

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,3 +175,34 @@ body: |
175175
$x2 = COPY %9(s64)
176176
RET_ReallyLR implicit $x0, implicit $x1, implicit $x2
177177
...
178+
...
179+
---
180+
name: uadde_opt_prev_dead
181+
alignment: 4
182+
legalized: true
183+
regBankSelected: true
184+
tracksRegLiveness: true
185+
body: |
186+
bb.1:
187+
liveins: $x0, $x1, $x2, $x3
188+
; CHECK-LABEL: name: uadde_opt_prev_dead
189+
; CHECK: liveins: $x0, $x1, $x2, $x3
190+
; CHECK-NEXT: {{ $}}
191+
; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr64 = COPY $x0
192+
; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr64 = COPY $x1
193+
; CHECK-NEXT: [[COPY2:%[0-9]+]]:gpr64 = COPY $x2
194+
; CHECK-NEXT: [[COPY3:%[0-9]+]]:gpr64 = COPY $x3
195+
; CHECK-NEXT: [[ADDSXrr:%[0-9]+]]:gpr64 = ADDSXrr [[COPY]], [[COPY2]], implicit-def $nzcv
196+
; CHECK-NEXT: [[ADCSXr:%[0-9]+]]:gpr64 = ADCSXr [[COPY1]], [[COPY3]], implicit-def $nzcv, implicit $nzcv
197+
; CHECK-NEXT: [[CSINCWr:%[0-9]+]]:gpr32 = CSINCWr $wzr, $wzr, 3, implicit $nzcv
198+
; CHECK-NEXT: $w0 = COPY [[CSINCWr]]
199+
; CHECK-NEXT: RET_ReallyLR implicit $w0
200+
%0:gpr(s64) = COPY $x0
201+
%1:gpr(s64) = COPY $x1
202+
%2:gpr(s64) = COPY $x2
203+
%3:gpr(s64) = COPY $x3
204+
%4:gpr(s64), %5:gpr(s32) = G_UADDO %0, %2
205+
%6:gpr(s64), %7:gpr(s32) = G_UADDE %1, %3, %5
206+
$w0 = COPY %7(s32)
207+
RET_ReallyLR implicit $w0
208+
...

‎llvm/test/CodeGen/AArch64/GlobalISel/select-usube.mir

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,3 +175,34 @@ body: |
175175
$x2 = COPY %9(s64)
176176
RET_ReallyLR implicit $x0, implicit $x1, implicit $x2
177177
...
178+
...
179+
---
180+
name: usube_opt_prev_dead
181+
alignment: 4
182+
legalized: true
183+
regBankSelected: true
184+
tracksRegLiveness: true
185+
body: |
186+
bb.1:
187+
liveins: $x0, $x1, $x2, $x3
188+
; CHECK-LABEL: name: usube_opt_prev_dead
189+
; CHECK: liveins: $x0, $x1, $x2, $x3
190+
; CHECK-NEXT: {{ $}}
191+
; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr64 = COPY $x0
192+
; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr64 = COPY $x1
193+
; CHECK-NEXT: [[COPY2:%[0-9]+]]:gpr64 = COPY $x2
194+
; CHECK-NEXT: [[COPY3:%[0-9]+]]:gpr64 = COPY $x3
195+
; CHECK-NEXT: [[SUBSXrr:%[0-9]+]]:gpr64 = SUBSXrr [[COPY]], [[COPY2]], implicit-def $nzcv
196+
; CHECK-NEXT: [[SBCSXr:%[0-9]+]]:gpr64 = SBCSXr [[COPY1]], [[COPY3]], implicit-def $nzcv, implicit $nzcv
197+
; CHECK-NEXT: [[CSINCWr:%[0-9]+]]:gpr32 = CSINCWr $wzr, $wzr, 2, implicit $nzcv
198+
; CHECK-NEXT: $w0 = COPY [[CSINCWr]]
199+
; CHECK-NEXT: RET_ReallyLR implicit $w0
200+
%0:gpr(s64) = COPY $x0
201+
%1:gpr(s64) = COPY $x1
202+
%2:gpr(s64) = COPY $x2
203+
%3:gpr(s64) = COPY $x3
204+
%4:gpr(s64), %5:gpr(s32) = G_USUBO %0, %2
205+
%6:gpr(s64), %7:gpr(s32) = G_USUBE %1, %3, %5
206+
$w0 = COPY %7(s32)
207+
RET_ReallyLR implicit $w0
208+
...

0 commit comments

Comments
 (0)