Skip to content

X86: Fix convertToThreeAddress losing subregister indexes #124098

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 18, 2025
Merged

X86: Fix convertToThreeAddress losing subregister indexes #124098

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
03ecc8d
X86: Fix convertToThreeAddress losing subregister indexes
arsenm Jan 23, 2025
065046b
Address addRegReg fixme
arsenm Feb 18, 2025
d15163e
fix
arsenm Feb 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion llvm/lib/Target/X86/X86FastISel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
13 changes: 8 additions & 5 deletions llvm/lib/Target/X86/X86InstrBuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 &
Expand Down
88 changes: 53 additions & 35 deletions llvm/lib/Target/X86/X86InstrInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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))
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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))
Expand All @@ -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))
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}

Expand All @@ -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) {
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down
5 changes: 3 additions & 2 deletions llvm/lib/Target/X86/X86InstrInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down