[llvm] 1f6165e - X86: Fix convertToThreeAddress losing subregister indexes (#124098)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 18 10:27:22 PST 2025
Author: Matt Arsenault
Date: 2025-02-19T01:27:19+07:00
New Revision: 1f6165e1844a69a726b92cf8821a645b0fff5303
URL: https://github.com/llvm/llvm-project/commit/1f6165e1844a69a726b92cf8821a645b0fff5303
DIFF: https://github.com/llvm/llvm-project/commit/1f6165e1844a69a726b92cf8821a645b0fff5303.diff
LOG: X86: Fix convertToThreeAddress losing subregister indexes (#124098)
This avoids dozens of regressions in a future patch. These
primarily manifested as assertions where we had copies of 64-bit
registers to 32-bit registers.
This is testable in principle with hand written MIR, but that's
a bit too much x86 for me.
Added:
Modified:
llvm/lib/Target/X86/X86FastISel.cpp
llvm/lib/Target/X86/X86InstrBuilder.h
llvm/lib/Target/X86/X86InstrInfo.cpp
llvm/lib/Target/X86/X86InstrInfo.h
Removed:
################################################################################
diff --git a/llvm/lib/Target/X86/X86FastISel.cpp b/llvm/lib/Target/X86/X86FastISel.cpp
index 039b8929c93a8..23de30275e2a1 100644
--- a/llvm/lib/Target/X86/X86FastISel.cpp
+++ b/llvm/lib/Target/X86/X86FastISel.cpp
@@ -3830,7 +3830,8 @@ unsigned X86FastISel::X86MaterializeFP(const ConstantFP *CFP, MVT VT) {
.addConstantPoolIndex(CPI, 0, OpFlag);
MachineInstrBuilder MIB = BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD,
TII.get(Opc), ResultReg);
- addRegReg(MIB, AddrReg, false, PICBase, false);
+ addRegReg(MIB, AddrReg, false, X86::NoSubRegister, PICBase, false,
+ X86::NoSubRegister);
MachineMemOperand *MMO = FuncInfo.MF->getMachineMemOperand(
MachinePointerInfo::getConstantPool(*FuncInfo.MF),
MachineMemOperand::MOLoad, DL.getPointerSize(), Alignment);
diff --git a/llvm/lib/Target/X86/X86InstrBuilder.h b/llvm/lib/Target/X86/X86InstrBuilder.h
index 07079ef87fd46..45c5f8aa82e97 100644
--- a/llvm/lib/Target/X86/X86InstrBuilder.h
+++ b/llvm/lib/Target/X86/X86InstrBuilder.h
@@ -161,11 +161,14 @@ addRegOffset(const MachineInstrBuilder &MIB,
/// addRegReg - This function is used to add a memory reference of the form:
/// [Reg + Reg].
-static inline const MachineInstrBuilder &addRegReg(const MachineInstrBuilder &MIB,
- unsigned Reg1, bool isKill1,
- unsigned Reg2, bool isKill2) {
- return MIB.addReg(Reg1, getKillRegState(isKill1)).addImm(1)
- .addReg(Reg2, getKillRegState(isKill2)).addImm(0).addReg(0);
+static inline const MachineInstrBuilder &
+addRegReg(const MachineInstrBuilder &MIB, unsigned Reg1, bool isKill1,
+ unsigned SubReg1, unsigned Reg2, bool isKill2, unsigned SubReg2) {
+ return MIB.addReg(Reg1, getKillRegState(isKill1), SubReg1)
+ .addImm(1)
+ .addReg(Reg2, getKillRegState(isKill2), SubReg2)
+ .addImm(0)
+ .addReg(0);
}
static inline const MachineInstrBuilder &
diff --git a/llvm/lib/Target/X86/X86InstrInfo.cpp b/llvm/lib/Target/X86/X86InstrInfo.cpp
index 44db5b6865c42..d756e73659a24 100644
--- a/llvm/lib/Target/X86/X86InstrInfo.cpp
+++ b/llvm/lib/Target/X86/X86InstrInfo.cpp
@@ -1158,8 +1158,9 @@ static bool findRedundantFlagInstr(MachineInstr &CmpInstr,
bool X86InstrInfo::classifyLEAReg(MachineInstr &MI, const MachineOperand &Src,
unsigned Opc, bool AllowSP, Register &NewSrc,
- bool &isKill, MachineOperand &ImplicitOp,
- LiveVariables *LV, LiveIntervals *LIS) const {
+ unsigned &NewSrcSubReg, bool &isKill,
+ MachineOperand &ImplicitOp, LiveVariables *LV,
+ LiveIntervals *LIS) const {
MachineFunction &MF = *MI.getParent()->getParent();
const TargetRegisterClass *RC;
if (AllowSP) {
@@ -1168,12 +1169,16 @@ bool X86InstrInfo::classifyLEAReg(MachineInstr &MI, const MachineOperand &Src,
RC = Opc != X86::LEA32r ? &X86::GR64_NOSPRegClass : &X86::GR32_NOSPRegClass;
}
Register SrcReg = Src.getReg();
+ unsigned SubReg = Src.getSubReg();
isKill = MI.killsRegister(SrcReg, /*TRI=*/nullptr);
+ NewSrcSubReg = X86::NoSubRegister;
+
// For both LEA64 and LEA32 the register already has essentially the right
// type (32-bit or 64-bit) we may just need to forbid SP.
if (Opc != X86::LEA64_32r) {
NewSrc = SrcReg;
+ NewSrcSubReg = SubReg;
assert(!Src.isUndef() && "Undef op doesn't need optimization");
if (NewSrc.isVirtual() && !MF.getRegInfo().constrainRegClass(NewSrc, RC))
@@ -1189,16 +1194,18 @@ bool X86InstrInfo::classifyLEAReg(MachineInstr &MI, const MachineOperand &Src,
ImplicitOp.setImplicit();
NewSrc = getX86SubSuperRegister(SrcReg, 64);
+ assert(!SubReg && "no superregister for source");
assert(NewSrc.isValid() && "Invalid Operand");
assert(!Src.isUndef() && "Undef op doesn't need optimization");
} else {
// Virtual register of the wrong class, we have to create a temporary 64-bit
// vreg to feed into the LEA.
NewSrc = MF.getRegInfo().createVirtualRegister(RC);
+ NewSrcSubReg = X86::NoSubRegister;
MachineInstr *Copy =
BuildMI(*MI.getParent(), MI, MI.getDebugLoc(), get(TargetOpcode::COPY))
.addReg(NewSrc, RegState::Define | RegState::Undef, X86::sub_32bit)
- .addReg(SrcReg, getKillRegState(isKill));
+ .addReg(SrcReg, getKillRegState(isKill), SubReg);
// Which is obviously going to be dead after we're done with it.
isKill = true;
@@ -1258,7 +1265,9 @@ MachineInstr *X86InstrInfo::convertToThreeAddressWithLEA(unsigned MIOpc,
MachineBasicBlock::iterator MBBI = MI.getIterator();
Register Dest = MI.getOperand(0).getReg();
Register Src = MI.getOperand(1).getReg();
+ unsigned SrcSubReg = MI.getOperand(1).getSubReg();
Register Src2;
+ unsigned Src2SubReg;
bool IsDead = MI.getOperand(0).isDead();
bool IsKill = MI.getOperand(1).isKill();
unsigned SubReg = Is8BitOp ? X86::sub_8bit : X86::sub_16bit;
@@ -1268,7 +1277,7 @@ MachineInstr *X86InstrInfo::convertToThreeAddressWithLEA(unsigned MIOpc,
MachineInstr *InsMI =
BuildMI(MBB, MBBI, MI.getDebugLoc(), get(TargetOpcode::COPY))
.addReg(InRegLEA, RegState::Define, SubReg)
- .addReg(Src, getKillRegState(IsKill));
+ .addReg(Src, getKillRegState(IsKill), SrcSubReg);
MachineInstr *ImpDef2 = nullptr;
MachineInstr *InsMI2 = nullptr;
@@ -1306,12 +1315,14 @@ MachineInstr *X86InstrInfo::convertToThreeAddressWithLEA(unsigned MIOpc,
case X86::ADD16rr:
case X86::ADD16rr_DB: {
Src2 = MI.getOperand(2).getReg();
+ Src2SubReg = MI.getOperand(2).getSubReg();
bool IsKill2 = MI.getOperand(2).isKill();
assert(!MI.getOperand(2).isUndef() && "Undef op doesn't need optimization");
if (Src == Src2) {
// ADD8rr/ADD16rr killed %reg1028, %reg1028
// just a single insert_subreg.
- addRegReg(MIB, InRegLEA, true, InRegLEA, false);
+ addRegReg(MIB, InRegLEA, true, X86::NoSubRegister, InRegLEA, false,
+ X86::NoSubRegister);
} else {
if (Subtarget.is64Bit())
InRegLEA2 = RegInfo.createVirtualRegister(&X86::GR64_NOSPRegClass);
@@ -1323,8 +1334,9 @@ MachineInstr *X86InstrInfo::convertToThreeAddressWithLEA(unsigned MIOpc,
InRegLEA2);
InsMI2 = BuildMI(MBB, &*MIB, MI.getDebugLoc(), get(TargetOpcode::COPY))
.addReg(InRegLEA2, RegState::Define, SubReg)
- .addReg(Src2, getKillRegState(IsKill2));
- addRegReg(MIB, InRegLEA, true, InRegLEA2, true);
+ .addReg(Src2, getKillRegState(IsKill2), Src2SubReg);
+ addRegReg(MIB, InRegLEA, true, X86::NoSubRegister, InRegLEA2, true,
+ X86::NoSubRegister);
}
if (LV && IsKill2 && InsMI2)
LV->replaceKillInstruction(Src2, MI, *InsMI2);
@@ -1428,6 +1440,7 @@ MachineInstr *X86InstrInfo::convertToThreeAddress(MachineInstr &MI,
MachineInstr *NewMI = nullptr;
Register SrcReg, SrcReg2;
+ unsigned SrcSubReg, SrcSubReg2;
bool Is64Bit = Subtarget.is64Bit();
bool Is8BitOp = false;
@@ -1467,17 +1480,18 @@ MachineInstr *X86InstrInfo::convertToThreeAddress(MachineInstr &MI,
// LEA can't handle ESP.
bool isKill;
MachineOperand ImplicitOp = MachineOperand::CreateReg(0, false);
- if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/false, SrcReg, isKill,
- ImplicitOp, LV, LIS))
+ if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/false, SrcReg, SrcSubReg,
+ isKill, ImplicitOp, LV, LIS))
return nullptr;
- MachineInstrBuilder MIB = BuildMI(MF, MI.getDebugLoc(), get(Opc))
- .add(Dest)
- .addReg(0)
- .addImm(1LL << ShAmt)
- .addReg(SrcReg, getKillRegState(isKill))
- .addImm(0)
- .addReg(0);
+ MachineInstrBuilder MIB =
+ BuildMI(MF, MI.getDebugLoc(), get(Opc))
+ .add(Dest)
+ .addReg(0)
+ .addImm(1LL << ShAmt)
+ .addReg(SrcReg, getKillRegState(isKill), SrcSubReg)
+ .addImm(0)
+ .addReg(0);
if (ImplicitOp.getReg() != 0)
MIB.add(ImplicitOp);
NewMI = MIB;
@@ -1505,8 +1519,8 @@ MachineInstr *X86InstrInfo::convertToThreeAddress(MachineInstr &MI,
: (Is64Bit ? X86::LEA64_32r : X86::LEA32r);
bool isKill;
MachineOperand ImplicitOp = MachineOperand::CreateReg(0, false);
- if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/false, SrcReg, isKill,
- ImplicitOp, LV, LIS))
+ if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/false, SrcReg, SrcSubReg,
+ isKill, ImplicitOp, LV, LIS))
return nullptr;
MachineInstrBuilder MIB = BuildMI(MF, MI.getDebugLoc(), get(Opc))
@@ -1531,8 +1545,8 @@ MachineInstr *X86InstrInfo::convertToThreeAddress(MachineInstr &MI,
bool isKill;
MachineOperand ImplicitOp = MachineOperand::CreateReg(0, false);
- if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/false, SrcReg, isKill,
- ImplicitOp, LV, LIS))
+ if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/false, SrcReg, SrcSubReg,
+ isKill, ImplicitOp, LV, LIS))
return nullptr;
MachineInstrBuilder MIB = BuildMI(MF, MI.getDebugLoc(), get(Opc))
@@ -1569,8 +1583,8 @@ MachineInstr *X86InstrInfo::convertToThreeAddress(MachineInstr &MI,
const MachineOperand &Src2 = MI.getOperand(2);
bool isKill2;
MachineOperand ImplicitOp2 = MachineOperand::CreateReg(0, false);
- if (!classifyLEAReg(MI, Src2, Opc, /*AllowSP=*/false, SrcReg2, isKill2,
- ImplicitOp2, LV, LIS))
+ if (!classifyLEAReg(MI, Src2, Opc, /*AllowSP=*/false, SrcReg2, SrcSubReg2,
+ isKill2, ImplicitOp2, LV, LIS))
return nullptr;
bool isKill;
@@ -1580,9 +1594,10 @@ MachineInstr *X86InstrInfo::convertToThreeAddress(MachineInstr &MI,
// the first call inserted a COPY from Src2 and marked it as killed.
isKill = isKill2;
SrcReg = SrcReg2;
+ SrcSubReg = SrcSubReg2;
} else {
- if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/true, SrcReg, isKill,
- ImplicitOp, LV, LIS))
+ if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/true, SrcReg, SrcSubReg,
+ isKill, ImplicitOp, LV, LIS))
return nullptr;
}
@@ -1592,7 +1607,8 @@ MachineInstr *X86InstrInfo::convertToThreeAddress(MachineInstr &MI,
if (ImplicitOp2.getReg() != 0)
MIB.add(ImplicitOp2);
- NewMI = addRegReg(MIB, SrcReg, isKill, SrcReg2, isKill2);
+ NewMI =
+ addRegReg(MIB, SrcReg, isKill, SrcSubReg, SrcReg2, isKill2, SrcSubReg2);
// Add kills if classifyLEAReg created a new register.
if (LV) {
@@ -1625,13 +1641,14 @@ MachineInstr *X86InstrInfo::convertToThreeAddress(MachineInstr &MI,
bool isKill;
MachineOperand ImplicitOp = MachineOperand::CreateReg(0, false);
- if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/true, SrcReg, isKill,
- ImplicitOp, LV, LIS))
+ if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/true, SrcReg, SrcSubReg,
+ isKill, ImplicitOp, LV, LIS))
return nullptr;
- MachineInstrBuilder MIB = BuildMI(MF, MI.getDebugLoc(), get(Opc))
- .add(Dest)
- .addReg(SrcReg, getKillRegState(isKill));
+ MachineInstrBuilder MIB =
+ BuildMI(MF, MI.getDebugLoc(), get(Opc))
+ .add(Dest)
+ .addReg(SrcReg, getKillRegState(isKill), SrcSubReg);
if (ImplicitOp.getReg() != 0)
MIB.add(ImplicitOp);
@@ -1665,13 +1682,14 @@ MachineInstr *X86InstrInfo::convertToThreeAddress(MachineInstr &MI,
bool isKill;
MachineOperand ImplicitOp = MachineOperand::CreateReg(0, false);
- if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/true, SrcReg, isKill,
- ImplicitOp, LV, LIS))
+ if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/true, SrcReg, SrcSubReg,
+ isKill, ImplicitOp, LV, LIS))
return nullptr;
- MachineInstrBuilder MIB = BuildMI(MF, MI.getDebugLoc(), get(Opc))
- .add(Dest)
- .addReg(SrcReg, getKillRegState(isKill));
+ MachineInstrBuilder MIB =
+ BuildMI(MF, MI.getDebugLoc(), get(Opc))
+ .add(Dest)
+ .addReg(SrcReg, getKillRegState(isKill), SrcSubReg);
if (ImplicitOp.getReg() != 0)
MIB.add(ImplicitOp);
diff --git a/llvm/lib/Target/X86/X86InstrInfo.h b/llvm/lib/Target/X86/X86InstrInfo.h
index 5f87e02fe67c4..e499f925f48ec 100644
--- a/llvm/lib/Target/X86/X86InstrInfo.h
+++ b/llvm/lib/Target/X86/X86InstrInfo.h
@@ -310,8 +310,9 @@ class X86InstrInfo final : public X86GenInstrInfo {
/// operand to the LEA instruction.
bool classifyLEAReg(MachineInstr &MI, const MachineOperand &Src,
unsigned LEAOpcode, bool AllowSP, Register &NewSrc,
- bool &isKill, MachineOperand &ImplicitOp,
- LiveVariables *LV, LiveIntervals *LIS) const;
+ unsigned &NewSrcSubReg, bool &isKill,
+ MachineOperand &ImplicitOp, LiveVariables *LV,
+ LiveIntervals *LIS) const;
/// convertToThreeAddress - This method must be implemented by targets that
/// set the M_CONVERTIBLE_TO_3_ADDR flag. When this flag is set, the target
More information about the llvm-commits
mailing list