[llvm] fbf0a77 - [CodeGen] Avoid potential sideeffects from XOR (#67193)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 17 12:03:31 PDT 2023
Author: Bill Wendling
Date: 2023-10-17T12:03:26-07:00
New Revision: fbf0a77e80f18a6d0fd8a28833b0bc87a99b1b2f
URL: https://github.com/llvm/llvm-project/commit/fbf0a77e80f18a6d0fd8a28833b0bc87a99b1b2f
DIFF: https://github.com/llvm/llvm-project/commit/fbf0a77e80f18a6d0fd8a28833b0bc87a99b1b2f.diff
LOG: [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.
Added:
Modified:
llvm/include/llvm/CodeGen/TargetInstrInfo.h
llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
llvm/lib/Target/AArch64/AArch64InstrInfo.h
llvm/lib/Target/X86/X86InstrInfo.cpp
llvm/lib/Target/X86/X86InstrInfo.h
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
index 14e27abe882b03b..6c3e02b2f594050 100644
--- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
@@ -2093,10 +2093,13 @@ class TargetInstrInfo : public MCInstrInfo {
"Target didn't implement TargetInstrInfo::insertOutlinedCall!");
}
- /// Insert an architecture-specific instruction to clear a register.
+ /// Insert an architecture-specific instruction to clear a register. If you
+ /// need to avoid sideeffects (e.g. avoid XOR on x86, which sets EFLAGS), set
+ /// \p AllowSideEffects to \p false.
virtual void buildClearRegister(Register Reg, MachineBasicBlock &MBB,
MachineBasicBlock::iterator Iter,
- DebugLoc &DL) const {
+ DebugLoc &DL,
+ bool AllowSideEffects = true) const {
llvm_unreachable(
"Target didn't implement TargetInstrInfo::buildClearRegister!");
}
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
index 05c79b610cb36c5..7dcf24c26e124ae 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -9134,13 +9134,15 @@ bool AArch64InstrInfo::shouldOutlineFromFunctionByDefault(
void AArch64InstrInfo::buildClearRegister(Register Reg, MachineBasicBlock &MBB,
MachineBasicBlock::iterator Iter,
- DebugLoc &DL) const {
+ DebugLoc &DL,
+ bool AllowSideEffects) const {
const MachineFunction &MF = *MBB.getParent();
const AArch64Subtarget &STI = MF.getSubtarget<AArch64Subtarget>();
const AArch64RegisterInfo &TRI = *STI.getRegisterInfo();
if (TRI.isGeneralPurposeRegister(MF, Reg)) {
- BuildMI(MBB, Iter, DL, get(AArch64::MOVi64imm), Reg)
+ BuildMI(MBB, Iter, DL, get(AArch64::MOVZXi), Reg)
+ .addImm(0)
.addImm(0);
} else if (STI.hasSVE()) {
BuildMI(MBB, Iter, DL, get(AArch64::DUP_ZI_D), Reg)
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.h b/llvm/lib/Target/AArch64/AArch64InstrInfo.h
index 4a40b2fa122159f..a934103c90cbf92 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.h
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.h
@@ -333,8 +333,8 @@ class AArch64InstrInfo final : public AArch64GenInstrInfo {
bool shouldOutlineFromFunctionByDefault(MachineFunction &MF) const override;
void buildClearRegister(Register Reg, MachineBasicBlock &MBB,
- MachineBasicBlock::iterator Iter,
- DebugLoc &DL) const override;
+ MachineBasicBlock::iterator Iter, DebugLoc &DL,
+ bool AllowSideEffects = true) const override;
/// Returns the vector element size (B, H, S or D) of an SVE opcode.
uint64_t getElementSizeForOpcode(unsigned Opc) const;
diff --git a/llvm/lib/Target/X86/X86InstrInfo.cpp b/llvm/lib/Target/X86/X86InstrInfo.cpp
index f0c46419ab3516e..4c6854da0ada3d2 100644
--- a/llvm/lib/Target/X86/X86InstrInfo.cpp
+++ b/llvm/lib/Target/X86/X86InstrInfo.cpp
@@ -10130,27 +10130,36 @@ X86InstrInfo::insertOutlinedCall(Module &M, MachineBasicBlock &MBB,
return It;
}
-void X86InstrInfo::buildClearRegister(Register Reg,
- MachineBasicBlock &MBB,
+void X86InstrInfo::buildClearRegister(Register Reg, MachineBasicBlock &MBB,
MachineBasicBlock::iterator Iter,
- DebugLoc &DL) const {
+ DebugLoc &DL,
+ bool AllowSideEffects) const {
const MachineFunction &MF = *MBB.getParent();
const X86Subtarget &ST = MF.getSubtarget<X86Subtarget>();
const TargetRegisterInfo &TRI = getRegisterInfo();
if (ST.hasMMX() && X86::VR64RegClass.contains(Reg))
- // FIXME: Ignore MMX registers?
+ // FIXME: Should we ignore MMX registers?
return;
if (TRI.isGeneralPurposeRegister(MF, Reg)) {
- BuildMI(MBB, Iter, DL, get(X86::XOR32rr), Reg)
- .addReg(Reg, RegState::Undef)
- .addReg(Reg, RegState::Undef);
+ // Convert register to the 32-bit version. Both 'movl' and 'xorl' clear the
+ // upper bits of a 64-bit register automagically.
+ Reg = getX86SubSuperRegister(Reg, 32);
+
+ if (!AllowSideEffects)
+ // XOR affects flags, so use a MOV instead.
+ BuildMI(MBB, Iter, DL, get(X86::MOV32ri), Reg).addImm(0);
+ else
+ BuildMI(MBB, Iter, DL, get(X86::XOR32rr), Reg)
+ .addReg(Reg, RegState::Undef)
+ .addReg(Reg, RegState::Undef);
} else if (X86::VR128RegClass.contains(Reg)) {
// XMM#
if (!ST.hasSSE1())
return;
+ // PXOR is safe to use because it doesn't affect flags.
BuildMI(MBB, Iter, DL, get(X86::PXORrr), Reg)
.addReg(Reg, RegState::Undef)
.addReg(Reg, RegState::Undef);
@@ -10159,6 +10168,7 @@ void X86InstrInfo::buildClearRegister(Register Reg,
if (!ST.hasAVX())
return;
+ // VPXOR is safe to use because it doesn't affect flags.
BuildMI(MBB, Iter, DL, get(X86::VPXORrr), Reg)
.addReg(Reg, RegState::Undef)
.addReg(Reg, RegState::Undef);
@@ -10167,6 +10177,7 @@ void X86InstrInfo::buildClearRegister(Register Reg,
if (!ST.hasAVX512())
return;
+ // VPXORY is safe to use because it doesn't affect flags.
BuildMI(MBB, Iter, DL, get(X86::VPXORYrr), Reg)
.addReg(Reg, RegState::Undef)
.addReg(Reg, RegState::Undef);
@@ -10178,9 +10189,11 @@ void X86InstrInfo::buildClearRegister(Register Reg,
if (!ST.hasVLX())
return;
- BuildMI(MBB, Iter, DL, get(ST.hasBWI() ? X86::KXORQrr : X86::KXORWrr), Reg)
- .addReg(Reg, RegState::Undef)
- .addReg(Reg, RegState::Undef);
+ // KXOR is safe to use because it doesn't affect flags.
+ unsigned Op = ST.hasBWI() ? X86::KXORQrr : X86::KXORWrr;
+ BuildMI(MBB, Iter, DL, get(Op), Reg)
+ .addReg(Reg, RegState::Undef)
+ .addReg(Reg, RegState::Undef);
}
}
diff --git a/llvm/lib/Target/X86/X86InstrInfo.h b/llvm/lib/Target/X86/X86InstrInfo.h
index 4d261a803421c1f..e1199e20c318e24 100644
--- a/llvm/lib/Target/X86/X86InstrInfo.h
+++ b/llvm/lib/Target/X86/X86InstrInfo.h
@@ -583,8 +583,8 @@ class X86InstrInfo final : public X86GenInstrInfo {
outliner::Candidate &C) const override;
void buildClearRegister(Register Reg, MachineBasicBlock &MBB,
- MachineBasicBlock::iterator Iter,
- DebugLoc &DL) const override;
+ MachineBasicBlock::iterator Iter, DebugLoc &DL,
+ bool AllowSideEffects = true) const override;
bool verifyInstruction(const MachineInstr &MI,
StringRef &ErrInfo) const override;
More information about the llvm-commits
mailing list