Skip to content

Commit f369653

Browse files
dstenbdstenb
committed
Revert "[DebugInfo] Make describeLoadedValue() reg aware"
This reverts commit 3cd93a4. I'll recommit with a well-formatted arcanist commit message.
1 parent 3cd93a4 commit f369653

File tree

13 files changed

+71
-770
lines changed

13 files changed

+71
-770
lines changed

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

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -72,14 +72,6 @@ struct DestSourcePair {
7272
: Destination(&Dest), Source(&Src) {}
7373
};
7474

75-
/// Used to describe a register and immediate addition.
76-
struct RegImmPair {
77-
Register Reg;
78-
int64_t Imm;
79-
80-
RegImmPair(Register Reg, int64_t Imm) : Reg(Reg), Imm(Imm) {}
81-
};
82-
8375
//---------------------------------------------------------------------------
8476
///
8577
/// TargetInstrInfo - Interface to description of machine instruction set
@@ -958,11 +950,11 @@ class TargetInstrInfo : public MCInstrInfo {
958950
}
959951

960952
/// If the specific machine instruction is an instruction that adds an
961-
/// immediate value and a physical register, and stores the result in
962-
/// the given physical register \c Reg, return a pair of the source
963-
/// register and the offset which has been added.
964-
virtual Optional<RegImmPair> isAddImmediate(const MachineInstr &MI,
965-
Register Reg) const {
953+
/// immediate value to its source operand and stores it in destination,
954+
/// return destination and source registers as machine operands along with
955+
/// \c Offset which has been added.
956+
virtual Optional<DestSourcePair> isAddImmediate(const MachineInstr &MI,
957+
int64_t &Offset) const {
966958
return None;
967959
}
968960

@@ -1797,10 +1789,9 @@ class TargetInstrInfo : public MCInstrInfo {
17971789
}
17981790

17991791
/// Produce the expression describing the \p MI loading a value into
1800-
/// the physical register \p Reg. This hook should only be used with
1801-
/// \p MIs belonging to VReg-less functions.
1802-
virtual Optional<ParamLoadedValue> describeLoadedValue(const MachineInstr &MI,
1803-
Register Reg) const;
1792+
/// the parameter's forwarding register.
1793+
virtual Optional<ParamLoadedValue>
1794+
describeLoadedValue(const MachineInstr &MI) const;
18041795

18051796
private:
18061797
unsigned CallFrameSetupOpcode, CallFrameDestroyOpcode;

‎llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp‎

Lines changed: 33 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,7 @@ static void collectCallSiteParameters(const MachineInstr *CallMI,
595595
Implicit.push_back(FwdReg);
596596
else
597597
Explicit.push_back(FwdReg);
598+
break;
598599
}
599600
}
600601
}
@@ -639,33 +640,39 @@ static void collectCallSiteParameters(const MachineInstr *CallMI,
639640
for (auto Reg : concat<unsigned>(ExplicitFwdRegDefs, ImplicitFwdRegDefs))
640641
ForwardedRegWorklist.erase(Reg);
641642

642-
for (auto ParamFwdReg : ExplicitFwdRegDefs) {
643-
if (auto ParamValue = TII->describeLoadedValue(*I, ParamFwdReg)) {
644-
if (ParamValue->first.isImm()) {
645-
int64_t Val = ParamValue->first.getImm();
646-
DbgValueLoc DbgLocVal(ParamValue->second, Val);
643+
// The describeLoadedValue() hook currently does not have any information
644+
// about which register it should describe in case of multiple defines, so
645+
// for now we only handle instructions where a forwarded register is (at
646+
// least partially) defined by the instruction's single explicit define.
647+
if (I->getNumExplicitDefs() != 1 || ExplicitFwdRegDefs.empty())
648+
continue;
649+
unsigned ParamFwdReg = ExplicitFwdRegDefs[0];
650+
651+
if (auto ParamValue = TII->describeLoadedValue(*I)) {
652+
if (ParamValue->first.isImm()) {
653+
int64_t Val = ParamValue->first.getImm();
654+
DbgValueLoc DbgLocVal(ParamValue->second, Val);
655+
finishCallSiteParam(DbgLocVal, ParamFwdReg);
656+
} else if (ParamValue->first.isReg()) {
657+
Register RegLoc = ParamValue->first.getReg();
658+
// TODO: For now, there is no use of describing the value loaded into the
659+
// register that is also the source registers (e.g. $r0 = add $r0, x).
660+
if (ParamFwdReg == RegLoc)
661+
continue;
662+
663+
unsigned SP = TLI->getStackPointerRegisterToSaveRestore();
664+
Register FP = TRI->getFrameRegister(*MF);
665+
bool IsSPorFP = (RegLoc == SP) || (RegLoc == FP);
666+
if (TRI->isCalleeSavedPhysReg(RegLoc, *MF) || IsSPorFP) {
667+
DbgValueLoc DbgLocVal(ParamValue->second,
668+
MachineLocation(RegLoc,
669+
/*IsIndirect=*/IsSPorFP));
647670
finishCallSiteParam(DbgLocVal, ParamFwdReg);
648-
} else if (ParamValue->first.isReg()) {
649-
Register RegLoc = ParamValue->first.getReg();
650-
// TODO: For now, there is no use of describing the value loaded into the
651-
// register that is also the source registers (e.g. $r0 = add $r0, x).
652-
if (ParamFwdReg == RegLoc)
653-
continue;
654-
655-
unsigned SP = TLI->getStackPointerRegisterToSaveRestore();
656-
Register FP = TRI->getFrameRegister(*MF);
657-
bool IsSPorFP = (RegLoc == SP) || (RegLoc == FP);
658-
if (TRI->isCalleeSavedPhysReg(RegLoc, *MF) || IsSPorFP) {
659-
DbgValueLoc DbgLocVal(ParamValue->second,
660-
MachineLocation(RegLoc,
661-
/*IsIndirect=*/IsSPorFP));
662-
finishCallSiteParam(DbgLocVal, ParamFwdReg);
663-
// TODO: Add support for entry value plus an expression.
664-
} else if (ShouldTryEmitEntryVals &&
665-
ParamValue->second->getNumElements() == 0) {
666-
ForwardedRegWorklist.insert(RegLoc);
667-
RegsForEntryValues[RegLoc] = ParamFwdReg;
668-
}
671+
// TODO: Add support for entry value plus an expression.
672+
} else if (ShouldTryEmitEntryVals &&
673+
ParamValue->second->getNumElements() == 0) {
674+
ForwardedRegWorklist.insert(RegLoc);
675+
RegsForEntryValues[RegLoc] = ParamFwdReg;
669676
}
670677
}
671678
}

‎llvm/lib/CodeGen/TargetInstrInfo.cpp‎

Lines changed: 5 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,35 +1121,16 @@ bool TargetInstrInfo::hasLowDefLatency(const TargetSchedModel &SchedModel,
11211121
}
11221122

11231123
Optional<ParamLoadedValue>
1124-
TargetInstrInfo::describeLoadedValue(const MachineInstr &MI,
1125-
Register Reg) const {
1124+
TargetInstrInfo::describeLoadedValue(const MachineInstr &MI) const {
11261125
const MachineFunction *MF = MI.getMF();
1127-
const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo();
11281126
DIExpression *Expr = DIExpression::get(MF->getFunction().getContext(), {});
11291127
int64_t Offset;
11301128

1131-
// To simplify the sub-register handling, verify that we only need to
1132-
// consider physical registers.
1133-
assert(MF->getProperties().hasProperty(
1134-
MachineFunctionProperties::Property::NoVRegs));
1135-
11361129
if (auto DestSrc = isCopyInstr(MI)) {
1137-
Register DestReg = DestSrc->Destination->getReg();
1138-
1139-
if (Reg == DestReg)
1140-
return ParamLoadedValue(*DestSrc->Source, Expr);
1141-
1142-
// Cases where super- or sub-registers needs to be described should
1143-
// be handled by the target's hook implementation.
1144-
assert(!TRI->isSuperOrSubRegisterEq(Reg, DestReg) &&
1145-
"TargetInstrInfo::describeLoadedValue can't describe super- or "
1146-
"sub-regs for copy instructions");
1147-
return None;
1148-
} else if (auto RegImm = isAddImmediate(MI, Reg)) {
1149-
Register SrcReg = RegImm->Reg;
1150-
Offset = RegImm->Imm;
1130+
return ParamLoadedValue(*DestSrc->Source, Expr);
1131+
} else if (auto DestSrc = isAddImmediate(MI, Offset)) {
11511132
Expr = DIExpression::prepend(Expr, DIExpression::ApplyOffset, Offset);
1152-
return ParamLoadedValue(MachineOperand::CreateReg(SrcReg, false), Expr);
1133+
return ParamLoadedValue(*DestSrc->Source, Expr);
11531134
} else if (MI.hasOneMemOperand()) {
11541135
// Only describe memory which provably does not escape the function. As
11551136
// described in llvm.org/PR43343, escaped memory may be clobbered by the
@@ -1164,15 +1145,11 @@ TargetInstrInfo::describeLoadedValue(const MachineInstr &MI,
11641145
if (!PSV || PSV->mayAlias(&MFI))
11651146
return None;
11661147

1148+
const auto &TRI = MF->getSubtarget().getRegisterInfo();
11671149
const MachineOperand *BaseOp;
11681150
if (!TII->getMemOperandWithOffset(MI, BaseOp, Offset, TRI))
11691151
return None;
11701152

1171-
assert(MI.getNumExplicitDefs() == 1 &&
1172-
"Can currently only handle mem instructions with a single define");
1173-
1174-
// TODO: In what way do we need to take Reg into consideration here?
1175-
11761153
SmallVector<uint64_t, 8> Ops;
11771154
DIExpression::appendOffset(Ops, Offset);
11781155
Ops.push_back(dwarf::DW_OP_deref_size);

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

Lines changed: 7 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
#include "llvm/CodeGen/StackMaps.h"
3131
#include "llvm/CodeGen/TargetRegisterInfo.h"
3232
#include "llvm/CodeGen/TargetSubtargetInfo.h"
33-
#include "llvm/IR/DebugInfoMetadata.h"
3433
#include "llvm/IR/DebugLoc.h"
3534
#include "llvm/IR/GlobalValue.h"
3635
#include "llvm/MC/MCAsmInfo.h"
@@ -6448,16 +6447,10 @@ AArch64InstrInfo::isCopyInstrImpl(const MachineInstr &MI) const {
64486447
return None;
64496448
}
64506449

6451-
Optional<RegImmPair> AArch64InstrInfo::isAddImmediate(const MachineInstr &MI,
6452-
Register Reg) const {
6450+
Optional<DestSourcePair>
6451+
AArch64InstrInfo::isAddImmediate(const MachineInstr &MI,
6452+
int64_t &Offset) const {
64536453
int Sign = 1;
6454-
int64_t Offset = 0;
6455-
6456-
// TODO: Handle cases where Reg is a super- or sub-register of the
6457-
// destination register.
6458-
if (Reg != MI.getOperand(0).getReg())
6459-
return None;
6460-
64616454
switch (MI.getOpcode()) {
64626455
default:
64636456
return None;
@@ -6481,73 +6474,22 @@ Optional<RegImmPair> AArch64InstrInfo::isAddImmediate(const MachineInstr &MI,
64816474
Offset = Offset << Shift;
64826475
}
64836476
}
6484-
return RegImmPair{MI.getOperand(1).getReg(), Offset};
6485-
}
6486-
6487-
/// If the given ORR instruction is a copy, and \p DescribedReg overlaps with
6488-
/// the destination register then, if possible, describe the value in terms of
6489-
/// the source register.
6490-
static Optional<ParamLoadedValue>
6491-
describeORRLoadedValue(const MachineInstr &MI, Register DescribedReg,
6492-
const TargetInstrInfo *TII,
6493-
const TargetRegisterInfo *TRI) {
6494-
auto DestSrc = TII->isCopyInstr(MI);
6495-
if (!DestSrc)
6496-
return None;
6497-
6498-
Register DestReg = DestSrc->Destination->getReg();
6499-
Register SrcReg = DestSrc->Source->getReg();
6500-
6501-
auto Expr = DIExpression::get(MI.getMF()->getFunction().getContext(), {});
6502-
6503-
// If the described register is the destination, just return the source.
6504-
if (DestReg == DescribedReg)
6505-
return ParamLoadedValue(MachineOperand::CreateReg(SrcReg, false), Expr);
6506-
6507-
// ORRWrs zero-extends to 64-bits, so we need to consider such cases.
6508-
if (MI.getOpcode() == AArch64::ORRWrs &&
6509-
TRI->isSuperRegister(DestReg, DescribedReg))
6510-
return ParamLoadedValue(MachineOperand::CreateReg(SrcReg, false), Expr);
6511-
6512-
// We may need to describe the lower part of a ORRXrs move.
6513-
if (MI.getOpcode() == AArch64::ORRXrs &&
6514-
TRI->isSubRegister(DestReg, DescribedReg)) {
6515-
Register SrcSubReg = TRI->getSubReg(SrcReg, AArch64::sub_32);
6516-
return ParamLoadedValue(MachineOperand::CreateReg(SrcSubReg, false), Expr);
6517-
}
6518-
6519-
assert(!TRI->isSuperOrSubRegisterEq(DestReg, DescribedReg) &&
6520-
"Unhandled ORR[XW]rs copy case");
6521-
6522-
return None;
6477+
return DestSourcePair{MI.getOperand(0), MI.getOperand(1)};
65236478
}
65246479

65256480
Optional<ParamLoadedValue>
6526-
AArch64InstrInfo::describeLoadedValue(const MachineInstr &MI,
6527-
Register Reg) const {
6528-
const MachineFunction *MF = MI.getMF();
6529-
const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo();
6481+
AArch64InstrInfo::describeLoadedValue(const MachineInstr &MI) const {
65306482
switch (MI.getOpcode()) {
65316483
case AArch64::MOVZWi:
6532-
case AArch64::MOVZXi: {
6533-
// MOVZWi may be used for producing zero-extended 32-bit immediates in
6534-
// 64-bit parameters, so we need to consider super-registers.
6535-
if (!TRI->isSuperRegisterEq(MI.getOperand(0).getReg(), Reg))
6536-
return None;
6537-
6484+
case AArch64::MOVZXi:
65386485
if (!MI.getOperand(1).isImm())
65396486
return None;
65406487
int64_t Immediate = MI.getOperand(1).getImm();
65416488
int Shift = MI.getOperand(2).getImm();
65426489
return ParamLoadedValue(MachineOperand::CreateImm(Immediate << Shift),
65436490
nullptr);
65446491
}
6545-
case AArch64::ORRWrs:
6546-
case AArch64::ORRXrs:
6547-
return describeORRLoadedValue(MI, Reg, this, TRI);
6548-
}
6549-
6550-
return TargetInstrInfo::describeLoadedValue(MI, Reg);
6492+
return TargetInstrInfo::describeLoadedValue(MI);
65516493
}
65526494

65536495
#define GET_INSTRINFO_HELPERS

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -265,11 +265,11 @@ class AArch64InstrInfo final : public AArch64GenInstrInfo {
265265
/// on Windows.
266266
static bool isSEHInstruction(const MachineInstr &MI);
267267

268-
Optional<RegImmPair> isAddImmediate(const MachineInstr &MI,
269-
Register Reg) const override;
268+
Optional<DestSourcePair> isAddImmediate(const MachineInstr &MI,
269+
int64_t &Offset) const override;
270270

271-
Optional<ParamLoadedValue> describeLoadedValue(const MachineInstr &MI,
272-
Register Reg) const override;
271+
Optional<ParamLoadedValue>
272+
describeLoadedValue(const MachineInstr &MI) const override;
273273

274274
#define GET_INSTRINFO_HELPER_DECLS
275275
#include "AArch64GenInstrInfo.inc"

‎llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5328,16 +5328,11 @@ ARMBaseInstrInfo::getSerializableBitmaskMachineOperandTargetFlags() const {
53285328
return makeArrayRef(TargetFlags);
53295329
}
53305330

5331-
Optional<RegImmPair> ARMBaseInstrInfo::isAddImmediate(const MachineInstr &MI,
5332-
Register Reg) const {
5331+
Optional<DestSourcePair>
5332+
ARMBaseInstrInfo::isAddImmediate(const MachineInstr &MI,
5333+
int64_t &Offset) const {
53335334
int Sign = 1;
53345335
unsigned Opcode = MI.getOpcode();
5335-
int64_t Offset = 0;
5336-
5337-
// TODO: Handle cases where Reg is a super- or sub-register of the
5338-
// destination register.
5339-
if (Reg != MI.getOperand(0).getReg())
5340-
return None;
53415336

53425337
// We describe SUBri or ADDri instructions.
53435338
if (Opcode == ARM::SUBri)
@@ -5353,7 +5348,7 @@ Optional<RegImmPair> ARMBaseInstrInfo::isAddImmediate(const MachineInstr &MI,
53535348
return None;
53545349

53555350
Offset = MI.getOperand(2).getImm() * Sign;
5356-
return RegImmPair{MI.getOperand(1).getReg(), Offset};
5351+
return DestSourcePair{MI.getOperand(0), MI.getOperand(1)};
53575352
}
53585353

53595354
bool llvm::registerDefinedBetween(unsigned Reg,

‎llvm/lib/Target/ARM/ARMBaseInstrInfo.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -455,8 +455,8 @@ class ARMBaseInstrInfo : public ARMGenInstrInfo {
455455
return MI.getOperand(3).getReg();
456456
}
457457

458-
Optional<RegImmPair> isAddImmediate(const MachineInstr &MI,
459-
Register Reg) const override;
458+
Optional<DestSourcePair> isAddImmediate(const MachineInstr &MI,
459+
int64_t &Offset) const override;
460460
};
461461

462462
/// Get the operands corresponding to the given \p Pred value. By default, the

0 commit comments

Comments
 (0)