Skip to content

Commit a845061

Browse files
topperctopperc
authored
[AArch64] Use the same fast math preservation for MachineCombiner reassociation as X86/PowerPC/RISCV. (llvm#72820)
Don't blindly copy the original flags from the pre-reassociated instrutions. This copied the integer poison flags which are not safe to preserve after reassociation. For the FP flags, I think we should only keep the intersection of the flags. Override setSpecialOperandAttr to do this. Fixes llvm#72777.
1 parent 8840eb3 commit a845061

File tree

8 files changed

+48
-71
lines changed

8 files changed

+48
-71
lines changed

‎llvm/lib/CodeGen/TargetInstrInfo.cpp

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1122,8 +1122,7 @@ void TargetInstrInfo::reassociateOps(
11221122
MachineInstrBuilder MIB1 =
11231123
BuildMI(*MF, MIMetadata(Prev), TII->get(NewPrevOpc), NewVR)
11241124
.addReg(RegX, getKillRegState(KillX))
1125-
.addReg(RegY, getKillRegState(KillY))
1126-
.setMIFlags(Prev.getFlags());
1125+
.addReg(RegY, getKillRegState(KillY));
11271126

11281127
if (SwapRootOperands) {
11291128
std::swap(RegA, NewVR);
@@ -1133,8 +1132,21 @@ void TargetInstrInfo::reassociateOps(
11331132
MachineInstrBuilder MIB2 =
11341133
BuildMI(*MF, MIMetadata(Root), TII->get(NewRootOpc), RegC)
11351134
.addReg(RegA, getKillRegState(KillA))
1136-
.addReg(NewVR, getKillRegState(KillNewVR))
1137-
.setMIFlags(Root.getFlags());
1135+
.addReg(NewVR, getKillRegState(KillNewVR));
1136+
1137+
// Propagate FP flags from the original instructions.
1138+
// But clear poison-generating flags because those may not be valid now.
1139+
// TODO: There should be a helper function for copying only fast-math-flags.
1140+
uint32_t IntersectedFlags = Root.getFlags() & Prev.getFlags();
1141+
MIB1->setFlags(IntersectedFlags);
1142+
MIB1->clearFlag(MachineInstr::MIFlag::NoSWrap);
1143+
MIB1->clearFlag(MachineInstr::MIFlag::NoUWrap);
1144+
MIB1->clearFlag(MachineInstr::MIFlag::IsExact);
1145+
1146+
MIB2->setFlags(IntersectedFlags);
1147+
MIB2->clearFlag(MachineInstr::MIFlag::NoSWrap);
1148+
MIB2->clearFlag(MachineInstr::MIFlag::NoUWrap);
1149+
MIB2->clearFlag(MachineInstr::MIFlag::IsExact);
11381150

11391151
setSpecialOperandAttr(Root, Prev, *MIB1, *MIB2);
11401152

‎llvm/lib/Target/PowerPC/PPCInstrInfo.cpp

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -218,26 +218,6 @@ int PPCInstrInfo::getOperandLatency(const InstrItineraryData *ItinData,
218218
return Latency;
219219
}
220220

221-
/// This is an architecture-specific helper function of reassociateOps.
222-
/// Set special operand attributes for new instructions after reassociation.
223-
void PPCInstrInfo::setSpecialOperandAttr(MachineInstr &OldMI1,
224-
MachineInstr &OldMI2,
225-
MachineInstr &NewMI1,
226-
MachineInstr &NewMI2) const {
227-
// Propagate FP flags from the original instructions.
228-
// But clear poison-generating flags because those may not be valid now.
229-
uint32_t IntersectedFlags = OldMI1.getFlags() & OldMI2.getFlags();
230-
NewMI1.setFlags(IntersectedFlags);
231-
NewMI1.clearFlag(MachineInstr::MIFlag::NoSWrap);
232-
NewMI1.clearFlag(MachineInstr::MIFlag::NoUWrap);
233-
NewMI1.clearFlag(MachineInstr::MIFlag::IsExact);
234-
235-
NewMI2.setFlags(IntersectedFlags);
236-
NewMI2.clearFlag(MachineInstr::MIFlag::NoSWrap);
237-
NewMI2.clearFlag(MachineInstr::MIFlag::NoUWrap);
238-
NewMI2.clearFlag(MachineInstr::MIFlag::IsExact);
239-
}
240-
241221
void PPCInstrInfo::setSpecialOperandAttr(MachineInstr &MI,
242222
uint32_t Flags) const {
243223
MI.setFlags(Flags);

‎llvm/lib/Target/PowerPC/PPCInstrInfo.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -367,10 +367,6 @@ class PPCInstrInfo : public PPCGenInstrInfo {
367367
/// perserved for more FMA chain reassociations on PowerPC.
368368
int getExtendResourceLenLimit() const override { return 1; }
369369

370-
void setSpecialOperandAttr(MachineInstr &OldMI1, MachineInstr &OldMI2,
371-
MachineInstr &NewMI1,
372-
MachineInstr &NewMI2) const override;
373-
374370
// PowerPC specific version of setSpecialOperandAttr that copies Flags to MI
375371
// and clears nuw, nsw, and exact flags.
376372
void setSpecialOperandAttr(MachineInstr &MI, uint32_t Flags) const;

‎llvm/lib/Target/RISCV/RISCVInstrInfo.cpp

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1610,24 +1610,6 @@ MachineTraceStrategy RISCVInstrInfo::getMachineCombinerTraceStrategy() const {
16101610
return ForceMachineCombinerStrategy;
16111611
}
16121612

1613-
void RISCVInstrInfo::setSpecialOperandAttr(MachineInstr &OldMI1,
1614-
MachineInstr &OldMI2,
1615-
MachineInstr &NewMI1,
1616-
MachineInstr &NewMI2) const {
1617-
// Propagate FP flags from the original instructions.
1618-
// But clear poison-generating flags because those may not be valid now.
1619-
uint32_t IntersectedFlags = OldMI1.getFlags() & OldMI2.getFlags();
1620-
NewMI1.setFlags(IntersectedFlags);
1621-
NewMI1.clearFlag(MachineInstr::MIFlag::NoSWrap);
1622-
NewMI1.clearFlag(MachineInstr::MIFlag::NoUWrap);
1623-
NewMI1.clearFlag(MachineInstr::MIFlag::IsExact);
1624-
1625-
NewMI2.setFlags(IntersectedFlags);
1626-
NewMI2.clearFlag(MachineInstr::MIFlag::NoSWrap);
1627-
NewMI2.clearFlag(MachineInstr::MIFlag::NoUWrap);
1628-
NewMI2.clearFlag(MachineInstr::MIFlag::IsExact);
1629-
}
1630-
16311613
void RISCVInstrInfo::finalizeInsInstrs(
16321614
MachineInstr &Root, MachineCombinerPattern &P,
16331615
SmallVectorImpl<MachineInstr *> &InsInstrs) const {

‎llvm/lib/Target/RISCV/RISCVInstrInfo.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,9 +224,6 @@ class RISCVInstrInfo : public RISCVGenInstrInfo {
224224

225225
MachineTraceStrategy getMachineCombinerTraceStrategy() const override;
226226

227-
void setSpecialOperandAttr(MachineInstr &OldMI1, MachineInstr &OldMI2,
228-
MachineInstr &NewMI1,
229-
MachineInstr &NewMI2) const override;
230227
bool
231228
getMachineCombinerPatterns(MachineInstr &Root,
232229
SmallVectorImpl<MachineCombinerPattern> &Patterns,

‎llvm/lib/Target/X86/X86InstrInfo.cpp

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9597,20 +9597,6 @@ void X86InstrInfo::setSpecialOperandAttr(MachineInstr &OldMI1,
95979597
MachineInstr &OldMI2,
95989598
MachineInstr &NewMI1,
95999599
MachineInstr &NewMI2) const {
9600-
// Propagate FP flags from the original instructions.
9601-
// But clear poison-generating flags because those may not be valid now.
9602-
// TODO: There should be a helper function for copying only fast-math-flags.
9603-
uint32_t IntersectedFlags = OldMI1.getFlags() & OldMI2.getFlags();
9604-
NewMI1.setFlags(IntersectedFlags);
9605-
NewMI1.clearFlag(MachineInstr::MIFlag::NoSWrap);
9606-
NewMI1.clearFlag(MachineInstr::MIFlag::NoUWrap);
9607-
NewMI1.clearFlag(MachineInstr::MIFlag::IsExact);
9608-
9609-
NewMI2.setFlags(IntersectedFlags);
9610-
NewMI2.clearFlag(MachineInstr::MIFlag::NoSWrap);
9611-
NewMI2.clearFlag(MachineInstr::MIFlag::NoUWrap);
9612-
NewMI2.clearFlag(MachineInstr::MIFlag::IsExact);
9613-
96149600
// Integer instructions may define an implicit EFLAGS dest register operand.
96159601
MachineOperand *OldFlagDef1 = OldMI1.findRegisterDefOperand(X86::EFLAGS);
96169602
MachineOperand *OldFlagDef2 = OldMI2.findRegisterDefOperand(X86::EFLAGS);

‎llvm/test/CodeGen/AArch64/machine-combiner-reassociate.mir

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,11 @@ body: |
9191
9292
# Check that flags on the instructions are preserved after reassociation.
9393
# CHECK-LABEL: name: fadd_flags
94-
# CHECK: [[ADD1:%[0-9]+]]:fpr32 = nsz FADDSrr %0, %1, implicit $fpcr
95-
# CHECK-SAFE-NEXT: [[ADD2:%[0-9]+]]:fpr32 = nnan FADDSrr killed [[ADD1]], %2, implicit $fpcr
96-
# CHECK-SAFE-NEXT: [[ADD3:%[0-9]+]]:fpr32 = ninf FADDSrr killed [[ADD2]], %3, implicit $fpcr
97-
# CHECK-UNSAFE-NEXT: [[ADD2:%[0-9]+]]:fpr32 = nnan FADDSrr %2, %3, implicit $fpcr
98-
# CHECK-UNSAFE-NEXT: [[ADD3:%[0-9]+]]:fpr32 = ninf FADDSrr killed [[ADD1]], killed [[ADD2]], implicit $fpcr
94+
# CHECK: [[ADD1:%[0-9]+]]:fpr32 = nnan ninf nsz FADDSrr %0, %1, implicit $fpcr
95+
# CHECK-SAFE-NEXT: [[ADD2:%[0-9]+]]:fpr32 = nnan nsz FADDSrr killed [[ADD1]], %2, implicit $fpcr
96+
# CHECK-SAFE-NEXT: [[ADD3:%[0-9]+]]:fpr32 = ninf nsz FADDSrr killed [[ADD2]], %3, implicit $fpcr
97+
# CHECK-UNSAFE-NEXT: [[ADD2:%[0-9]+]]:fpr32 = nsz FADDSrr %2, %3, implicit $fpcr
98+
# CHECK-UNSAFE-NEXT: [[ADD3:%[0-9]+]]:fpr32 = nsz FADDSrr killed [[ADD1]], killed [[ADD2]], implicit $fpcr
9999
---
100100
name: fadd_flags
101101
alignment: 4
@@ -125,8 +125,8 @@ body: |
125125
%2:fpr32 = COPY $s2
126126
%1:fpr32 = COPY $s1
127127
%0:fpr32 = COPY $s0
128-
%4:fpr32 = nsz FADDSrr %0, %1, implicit $fpcr
129-
%5:fpr32 = nnan FADDSrr killed %4, %2, implicit $fpcr
130-
%6:fpr32 = ninf FADDSrr killed %5, %3, implicit $fpcr
128+
%4:fpr32 = nsz nnan ninf FADDSrr %0, %1, implicit $fpcr
129+
%5:fpr32 = nsz nnan FADDSrr killed %4, %2, implicit $fpcr
130+
%6:fpr32 = nsz ninf FADDSrr killed %5, %3, implicit $fpcr
131131
$s0 = COPY %6
132132
RET_ReallyLR implicit $s0

‎llvm/test/CodeGen/AArch64/pr72777.ll

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 4
2+
; RUN: llc < %s -mtriple=aarch64 | FileCheck %s
3+
4+
define i64 @f(i64 %0, i64 %1) {
5+
; CHECK-LABEL: f:
6+
; CHECK: // %bb.0:
7+
; CHECK-NEXT: orr x9, x1, #0x1
8+
; CHECK-NEXT: add x10, x0, x0
9+
; CHECK-NEXT: mov x8, #-9223372036854775808 // =0x8000000000000000
10+
; CHECK-NEXT: add x9, x9, x10
11+
; CHECK-NEXT: lsl x10, x9, #1
12+
; CHECK-NEXT: cmp x9, #0
13+
; CHECK-NEXT: cinv x8, x8, ge
14+
; CHECK-NEXT: cmp x9, x10, asr #1
15+
; CHECK-NEXT: csel x0, x8, x10, ne
16+
; CHECK-NEXT: ret
17+
%3 = or i64 1, %1
18+
%4 = add i64 %3, %0
19+
%5 = add nsw i64 %4, %0
20+
%6 = call i64 @llvm.sshl.sat.i64(i64 %5, i64 1)
21+
ret i64 %6
22+
}
23+
24+
declare i64 @llvm.sshl.sat.i64(i64, i64)

0 commit comments

Comments
 (0)