Skip to content

Commit 1a18b8f

Browse files
arsenmarsenm
committed
X86: Fix convertToThreeAddress losing subregister indexes
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.
1 parent 46ffacc commit 1a18b8f

File tree

2 files changed

+47
-35
lines changed

2 files changed

+47
-35
lines changed

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

Lines changed: 44 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1158,8 +1158,9 @@ static bool findRedundantFlagInstr(MachineInstr &CmpInstr,
11581158

11591159
bool X86InstrInfo::classifyLEAReg(MachineInstr &MI, const MachineOperand &Src,
11601160
unsigned Opc, bool AllowSP, Register &NewSrc,
1161-
bool &isKill, MachineOperand &ImplicitOp,
1162-
LiveVariables *LV, LiveIntervals *LIS) const {
1161+
unsigned &NewSrcSubReg, bool &isKill,
1162+
MachineOperand &ImplicitOp, LiveVariables *LV,
1163+
LiveIntervals *LIS) const {
11631164
MachineFunction &MF = *MI.getParent()->getParent();
11641165
const TargetRegisterClass *RC;
11651166
if (AllowSP) {
@@ -1168,12 +1169,14 @@ bool X86InstrInfo::classifyLEAReg(MachineInstr &MI, const MachineOperand &Src,
11681169
RC = Opc != X86::LEA32r ? &X86::GR64_NOSPRegClass : &X86::GR32_NOSPRegClass;
11691170
}
11701171
Register SrcReg = Src.getReg();
1172+
unsigned SubReg = Src.getSubReg();
11711173
isKill = MI.killsRegister(SrcReg, /*TRI=*/nullptr);
11721174

11731175
// For both LEA64 and LEA32 the register already has essentially the right
11741176
// type (32-bit or 64-bit) we may just need to forbid SP.
11751177
if (Opc != X86::LEA64_32r) {
11761178
NewSrc = SrcReg;
1179+
NewSrcSubReg = SubReg;
11771180
assert(!Src.isUndef() && "Undef op doesn't need optimization");
11781181

11791182
if (NewSrc.isVirtual() && !MF.getRegInfo().constrainRegClass(NewSrc, RC))
@@ -1189,6 +1192,7 @@ bool X86InstrInfo::classifyLEAReg(MachineInstr &MI, const MachineOperand &Src,
11891192
ImplicitOp.setImplicit();
11901193

11911194
NewSrc = getX86SubSuperRegister(SrcReg, 64);
1195+
assert(!SubReg && "no super register for source"");
11921196
assert(NewSrc.isValid() && "Invalid Operand");
11931197
assert(!Src.isUndef() && "Undef op doesn't need optimization");
11941198
} else {
@@ -1198,7 +1202,7 @@ bool X86InstrInfo::classifyLEAReg(MachineInstr &MI, const MachineOperand &Src,
11981202
MachineInstr *Copy =
11991203
BuildMI(*MI.getParent(), MI, MI.getDebugLoc(), get(TargetOpcode::COPY))
12001204
.addReg(NewSrc, RegState::Define | RegState::Undef, X86::sub_32bit)
1201-
.addReg(SrcReg, getKillRegState(isKill));
1205+
.addReg(SrcReg, getKillRegState(isKill), SubReg);
12021206
12031207
// Which is obviously going to be dead after we're done with it.
12041208
isKill = true;
@@ -1258,7 +1262,9 @@ MachineInstr *X86InstrInfo::convertToThreeAddressWithLEA(unsigned MIOpc,
12581262
MachineBasicBlock::iterator MBBI = MI.getIterator();
12591263
Register Dest = MI.getOperand(0).getReg();
12601264
Register Src = MI.getOperand(1).getReg();
1265+
unsigned SrcSubReg = MI.getOperand(1).getSubReg();
12611266
Register Src2;
1267+
unsigned Src2SubReg;
12621268
bool IsDead = MI.getOperand(0).isDead();
12631269
bool IsKill = MI.getOperand(1).isKill();
12641270
unsigned SubReg = Is8BitOp ? X86::sub_8bit : X86::sub_16bit;
@@ -1268,7 +1274,7 @@ MachineInstr *X86InstrInfo::convertToThreeAddressWithLEA(unsigned MIOpc,
12681274
MachineInstr *InsMI =
12691275
BuildMI(MBB, MBBI, MI.getDebugLoc(), get(TargetOpcode::COPY))
12701276
.addReg(InRegLEA, RegState::Define, SubReg)
1271-
.addReg(Src, getKillRegState(IsKill));
1277+
.addReg(Src, getKillRegState(IsKill), SrcSubReg);
12721278
MachineInstr *ImpDef2 = nullptr;
12731279
MachineInstr *InsMI2 = nullptr;
12741280

@@ -1306,6 +1312,7 @@ MachineInstr *X86InstrInfo::convertToThreeAddressWithLEA(unsigned MIOpc,
13061312
case X86::ADD16rr:
13071313
case X86::ADD16rr_DB: {
13081314
Src2 = MI.getOperand(2).getReg();
1315+
Src2SubReg = MI.getOperand(2).getSubReg();
13091316
bool IsKill2 = MI.getOperand(2).isKill();
13101317
assert(!MI.getOperand(2).isUndef() && "Undef op doesn't need optimization");
13111318
if (Src == Src2) {
@@ -1323,7 +1330,7 @@ MachineInstr *X86InstrInfo::convertToThreeAddressWithLEA(unsigned MIOpc,
13231330
InRegLEA2);
13241331
InsMI2 = BuildMI(MBB, &*MIB, MI.getDebugLoc(), get(TargetOpcode::COPY))
13251332
.addReg(InRegLEA2, RegState::Define, SubReg)
1326-
.addReg(Src2, getKillRegState(IsKill2));
1333+
.addReg(Src2, getKillRegState(IsKill2), Src2SubReg);
13271334
addRegReg(MIB, InRegLEA, true, InRegLEA2, true);
13281335
}
13291336
if (LV && IsKill2 && InsMI2)
@@ -1428,6 +1435,7 @@ MachineInstr *X86InstrInfo::convertToThreeAddress(MachineInstr &MI,
14281435

14291436
MachineInstr *NewMI = nullptr;
14301437
Register SrcReg, SrcReg2;
1438+
unsigned SrcSubReg, SrcSubReg2;
14311439
bool Is64Bit = Subtarget.is64Bit();
14321440

14331441
bool Is8BitOp = false;
@@ -1467,17 +1475,18 @@ MachineInstr *X86InstrInfo::convertToThreeAddress(MachineInstr &MI,
14671475
// LEA can't handle ESP.
14681476
bool isKill;
14691477
MachineOperand ImplicitOp = MachineOperand::CreateReg(0, false);
1470-
if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/false, SrcReg, isKill,
1471-
ImplicitOp, LV, LIS))
1478+
if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/false, SrcReg, SrcSubReg,
1479+
isKill, ImplicitOp, LV, LIS))
14721480
return nullptr;
14731481

1474-
MachineInstrBuilder MIB = BuildMI(MF, MI.getDebugLoc(), get(Opc))
1475-
.add(Dest)
1476-
.addReg(0)
1477-
.addImm(1LL << ShAmt)
1478-
.addReg(SrcReg, getKillRegState(isKill))
1479-
.addImm(0)
1480-
.addReg(0);
1482+
MachineInstrBuilder MIB =
1483+
BuildMI(MF, MI.getDebugLoc(), get(Opc))
1484+
.add(Dest)
1485+
.addReg(0)
1486+
.addImm(1LL << ShAmt)
1487+
.addReg(SrcReg, getKillRegState(isKill), SrcSubReg)
1488+
.addImm(0)
1489+
.addReg(0);
14811490
if (ImplicitOp.getReg() != 0)
14821491
MIB.add(ImplicitOp);
14831492
NewMI = MIB;
@@ -1505,8 +1514,8 @@ MachineInstr *X86InstrInfo::convertToThreeAddress(MachineInstr &MI,
15051514
: (Is64Bit ? X86::LEA64_32r : X86::LEA32r);
15061515
bool isKill;
15071516
MachineOperand ImplicitOp = MachineOperand::CreateReg(0, false);
1508-
if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/false, SrcReg, isKill,
1509-
ImplicitOp, LV, LIS))
1517+
if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/false, SrcReg, SrcSubReg,
1518+
isKill, ImplicitOp, LV, LIS))
15101519
return nullptr;
15111520

15121521
MachineInstrBuilder MIB = BuildMI(MF, MI.getDebugLoc(), get(Opc))
@@ -1531,8 +1540,8 @@ MachineInstr *X86InstrInfo::convertToThreeAddress(MachineInstr &MI,
15311540

15321541
bool isKill;
15331542
MachineOperand ImplicitOp = MachineOperand::CreateReg(0, false);
1534-
if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/false, SrcReg, isKill,
1535-
ImplicitOp, LV, LIS))
1543+
if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/false, SrcReg, SrcSubReg,
1544+
isKill, ImplicitOp, LV, LIS))
15361545
return nullptr;
15371546

15381547
MachineInstrBuilder MIB = BuildMI(MF, MI.getDebugLoc(), get(Opc))
@@ -1569,8 +1578,8 @@ MachineInstr *X86InstrInfo::convertToThreeAddress(MachineInstr &MI,
15691578
const MachineOperand &Src2 = MI.getOperand(2);
15701579
bool isKill2;
15711580
MachineOperand ImplicitOp2 = MachineOperand::CreateReg(0, false);
1572-
if (!classifyLEAReg(MI, Src2, Opc, /*AllowSP=*/false, SrcReg2, isKill2,
1573-
ImplicitOp2, LV, LIS))
1581+
if (!classifyLEAReg(MI, Src2, Opc, /*AllowSP=*/false, SrcReg2, SrcSubReg2,
1582+
isKill2, ImplicitOp2, LV, LIS))
15741583
return nullptr;
15751584

15761585
bool isKill;
@@ -1581,8 +1590,8 @@ MachineInstr *X86InstrInfo::convertToThreeAddress(MachineInstr &MI,
15811590
isKill = isKill2;
15821591
SrcReg = SrcReg2;
15831592
} else {
1584-
if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/true, SrcReg, isKill,
1585-
ImplicitOp, LV, LIS))
1593+
if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/true, SrcReg, SrcSubReg,
1594+
isKill, ImplicitOp, LV, LIS))
15861595
return nullptr;
15871596
}
15881597

@@ -1592,7 +1601,7 @@ MachineInstr *X86InstrInfo::convertToThreeAddress(MachineInstr &MI,
15921601
if (ImplicitOp2.getReg() != 0)
15931602
MIB.add(ImplicitOp2);
15941603

1595-
NewMI = addRegReg(MIB, SrcReg, isKill, SrcReg2, isKill2);
1604+
NewMI = addRegReg(MIB, SrcReg, isKill, SrcReg2, isKill2); // FIXME: Subregs
15961605

15971606
// Add kills if classifyLEAReg created a new register.
15981607
if (LV) {
@@ -1625,13 +1634,14 @@ MachineInstr *X86InstrInfo::convertToThreeAddress(MachineInstr &MI,
16251634

16261635
bool isKill;
16271636
MachineOperand ImplicitOp = MachineOperand::CreateReg(0, false);
1628-
if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/true, SrcReg, isKill,
1629-
ImplicitOp, LV, LIS))
1637+
if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/true, SrcReg, SrcSubReg,
1638+
isKill, ImplicitOp, LV, LIS))
16301639
return nullptr;
16311640

1632-
MachineInstrBuilder MIB = BuildMI(MF, MI.getDebugLoc(), get(Opc))
1633-
.add(Dest)
1634-
.addReg(SrcReg, getKillRegState(isKill));
1641+
MachineInstrBuilder MIB =
1642+
BuildMI(MF, MI.getDebugLoc(), get(Opc))
1643+
.add(Dest)
1644+
.addReg(SrcReg, getKillRegState(isKill), SrcSubReg);
16351645
if (ImplicitOp.getReg() != 0)
16361646
MIB.add(ImplicitOp);
16371647

@@ -1665,13 +1675,14 @@ MachineInstr *X86InstrInfo::convertToThreeAddress(MachineInstr &MI,
16651675

16661676
bool isKill;
16671677
MachineOperand ImplicitOp = MachineOperand::CreateReg(0, false);
1668-
if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/true, SrcReg, isKill,
1669-
ImplicitOp, LV, LIS))
1678+
if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/true, SrcReg, SrcSubReg,
1679+
isKill, ImplicitOp, LV, LIS))
16701680
return nullptr;
16711681

1672-
MachineInstrBuilder MIB = BuildMI(MF, MI.getDebugLoc(), get(Opc))
1673-
.add(Dest)
1674-
.addReg(SrcReg, getKillRegState(isKill));
1682+
MachineInstrBuilder MIB =
1683+
BuildMI(MF, MI.getDebugLoc(), get(Opc))
1684+
.add(Dest)
1685+
.addReg(SrcReg, getKillRegState(isKill), SrcSubReg);
16751686
if (ImplicitOp.getReg() != 0)
16761687
MIB.add(ImplicitOp);
16771688

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -310,8 +310,9 @@ class X86InstrInfo final : public X86GenInstrInfo {
310310
/// operand to the LEA instruction.
311311
bool classifyLEAReg(MachineInstr &MI, const MachineOperand &Src,
312312
unsigned LEAOpcode, bool AllowSP, Register &NewSrc,
313-
bool &isKill, MachineOperand &ImplicitOp,
314-
LiveVariables *LV, LiveIntervals *LIS) const;
313+
unsigned &NewSrcSubReg, bool &isKill,
314+
MachineOperand &ImplicitOp, LiveVariables *LV,
315+
LiveIntervals *LIS) const;
315316

316317
/// convertToThreeAddress - This method must be implemented by targets that
317318
/// set the M_CONVERTIBLE_TO_3_ADDR flag. When this flag is set, the target

0 commit comments

Comments
 (0)