Skip to content

Commit fbf0a77

Browse files
bwendlingbwendling
authored
[CodeGen] Avoid potential sideeffects from XOR (#67193)
XOR may change flag values (e.g. for X86 gprs). In the case where that's not desirable, specify that buildClearRegister() should use MOV instead.
1 parent e6d0b12 commit fbf0a77

File tree

5 files changed

+36
-18
lines changed

5 files changed

+36
-18
lines changed

‎llvm/include/llvm/CodeGen/TargetInstrInfo.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2093,10 +2093,13 @@ class TargetInstrInfo : public MCInstrInfo {
20932093
"Target didn't implement TargetInstrInfo::insertOutlinedCall!");
20942094
}
20952095

2096-
/// Insert an architecture-specific instruction to clear a register.
2096+
/// Insert an architecture-specific instruction to clear a register. If you
2097+
/// need to avoid sideeffects (e.g. avoid XOR on x86, which sets EFLAGS), set
2098+
/// \p AllowSideEffects to \p false.
20972099
virtual void buildClearRegister(Register Reg, MachineBasicBlock &MBB,
20982100
MachineBasicBlock::iterator Iter,
2099-
DebugLoc &DL) const {
2101+
DebugLoc &DL,
2102+
bool AllowSideEffects = true) const {
21002103
llvm_unreachable(
21012104
"Target didn't implement TargetInstrInfo::buildClearRegister!");
21022105
}

‎llvm/lib/Target/AArch64/AArch64InstrInfo.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9134,13 +9134,15 @@ bool AArch64InstrInfo::shouldOutlineFromFunctionByDefault(
91349134

91359135
void AArch64InstrInfo::buildClearRegister(Register Reg, MachineBasicBlock &MBB,
91369136
MachineBasicBlock::iterator Iter,
9137-
DebugLoc &DL) const {
9137+
DebugLoc &DL,
9138+
bool AllowSideEffects) const {
91389139
const MachineFunction &MF = *MBB.getParent();
91399140
const AArch64Subtarget &STI = MF.getSubtarget<AArch64Subtarget>();
91409141
const AArch64RegisterInfo &TRI = *STI.getRegisterInfo();
91419142

91429143
if (TRI.isGeneralPurposeRegister(MF, Reg)) {
9143-
BuildMI(MBB, Iter, DL, get(AArch64::MOVi64imm), Reg)
9144+
BuildMI(MBB, Iter, DL, get(AArch64::MOVZXi), Reg)
9145+
.addImm(0)
91449146
.addImm(0);
91459147
} else if (STI.hasSVE()) {
91469148
BuildMI(MBB, Iter, DL, get(AArch64::DUP_ZI_D), Reg)

‎llvm/lib/Target/AArch64/AArch64InstrInfo.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -333,8 +333,8 @@ class AArch64InstrInfo final : public AArch64GenInstrInfo {
333333
bool shouldOutlineFromFunctionByDefault(MachineFunction &MF) const override;
334334

335335
void buildClearRegister(Register Reg, MachineBasicBlock &MBB,
336-
MachineBasicBlock::iterator Iter,
337-
DebugLoc &DL) const override;
336+
MachineBasicBlock::iterator Iter, DebugLoc &DL,
337+
bool AllowSideEffects = true) const override;
338338

339339
/// Returns the vector element size (B, H, S or D) of an SVE opcode.
340340
uint64_t getElementSizeForOpcode(unsigned Opc) const;

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

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10130,27 +10130,36 @@ X86InstrInfo::insertOutlinedCall(Module &M, MachineBasicBlock &MBB,
1013010130
return It;
1013110131
}
1013210132

10133-
void X86InstrInfo::buildClearRegister(Register Reg,
10134-
MachineBasicBlock &MBB,
10133+
void X86InstrInfo::buildClearRegister(Register Reg, MachineBasicBlock &MBB,
1013510134
MachineBasicBlock::iterator Iter,
10136-
DebugLoc &DL) const {
10135+
DebugLoc &DL,
10136+
bool AllowSideEffects) const {
1013710137
const MachineFunction &MF = *MBB.getParent();
1013810138
const X86Subtarget &ST = MF.getSubtarget<X86Subtarget>();
1013910139
const TargetRegisterInfo &TRI = getRegisterInfo();
1014010140

1014110141
if (ST.hasMMX() && X86::VR64RegClass.contains(Reg))
10142-
// FIXME: Ignore MMX registers?
10142+
// FIXME: Should we ignore MMX registers?
1014310143
return;
1014410144

1014510145
if (TRI.isGeneralPurposeRegister(MF, Reg)) {
10146-
BuildMI(MBB, Iter, DL, get(X86::XOR32rr), Reg)
10147-
.addReg(Reg, RegState::Undef)
10148-
.addReg(Reg, RegState::Undef);
10146+
// Convert register to the 32-bit version. Both 'movl' and 'xorl' clear the
10147+
// upper bits of a 64-bit register automagically.
10148+
Reg = getX86SubSuperRegister(Reg, 32);
10149+
10150+
if (!AllowSideEffects)
10151+
// XOR affects flags, so use a MOV instead.
10152+
BuildMI(MBB, Iter, DL, get(X86::MOV32ri), Reg).addImm(0);
10153+
else
10154+
BuildMI(MBB, Iter, DL, get(X86::XOR32rr), Reg)
10155+
.addReg(Reg, RegState::Undef)
10156+
.addReg(Reg, RegState::Undef);
1014910157
} else if (X86::VR128RegClass.contains(Reg)) {
1015010158
// XMM#
1015110159
if (!ST.hasSSE1())
1015210160
return;
1015310161

10162+
// PXOR is safe to use because it doesn't affect flags.
1015410163
BuildMI(MBB, Iter, DL, get(X86::PXORrr), Reg)
1015510164
.addReg(Reg, RegState::Undef)
1015610165
.addReg(Reg, RegState::Undef);
@@ -10159,6 +10168,7 @@ void X86InstrInfo::buildClearRegister(Register Reg,
1015910168
if (!ST.hasAVX())
1016010169
return;
1016110170

10171+
// VPXOR is safe to use because it doesn't affect flags.
1016210172
BuildMI(MBB, Iter, DL, get(X86::VPXORrr), Reg)
1016310173
.addReg(Reg, RegState::Undef)
1016410174
.addReg(Reg, RegState::Undef);
@@ -10167,6 +10177,7 @@ void X86InstrInfo::buildClearRegister(Register Reg,
1016710177
if (!ST.hasAVX512())
1016810178
return;
1016910179

10180+
// VPXORY is safe to use because it doesn't affect flags.
1017010181
BuildMI(MBB, Iter, DL, get(X86::VPXORYrr), Reg)
1017110182
.addReg(Reg, RegState::Undef)
1017210183
.addReg(Reg, RegState::Undef);
@@ -10178,9 +10189,11 @@ void X86InstrInfo::buildClearRegister(Register Reg,
1017810189
if (!ST.hasVLX())
1017910190
return;
1018010191

10181-
BuildMI(MBB, Iter, DL, get(ST.hasBWI() ? X86::KXORQrr : X86::KXORWrr), Reg)
10182-
.addReg(Reg, RegState::Undef)
10183-
.addReg(Reg, RegState::Undef);
10192+
// KXOR is safe to use because it doesn't affect flags.
10193+
unsigned Op = ST.hasBWI() ? X86::KXORQrr : X86::KXORWrr;
10194+
BuildMI(MBB, Iter, DL, get(Op), Reg)
10195+
.addReg(Reg, RegState::Undef)
10196+
.addReg(Reg, RegState::Undef);
1018410197
}
1018510198
}
1018610199

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -583,8 +583,8 @@ class X86InstrInfo final : public X86GenInstrInfo {
583583
outliner::Candidate &C) const override;
584584

585585
void buildClearRegister(Register Reg, MachineBasicBlock &MBB,
586-
MachineBasicBlock::iterator Iter,
587-
DebugLoc &DL) const override;
586+
MachineBasicBlock::iterator Iter, DebugLoc &DL,
587+
bool AllowSideEffects = true) const override;
588588

589589
bool verifyInstruction(const MachineInstr &MI,
590590
StringRef &ErrInfo) const override;

0 commit comments

Comments
 (0)