Skip to content

Commit b288f7d

Browse files
amykhuangamykhuang
committed
[codeview] Fix for PR43479
Summary: Add instruction marker to MachineInstr ExtraInfo. This does almost the same thing as Pre/PostInstrSymbols, except that it doesn't create a label until printing instructions. This allows for labels to be put around instructions that are deleted/duplicated somewhere. Use this marker to track heap alloc site call instructions. Reviewers: rnk Subscribers: MatzeB, hiraditya, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D69536 cherry picked from 7420430 with some modifications.
1 parent bc6d0f1 commit b288f7d

File tree

12 files changed

+336
-171
lines changed

12 files changed

+336
-171
lines changed

‎llvm/include/llvm/CodeGen/MachineFunction.h

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -787,10 +787,9 @@ class MachineFunction {
787787
///
788788
/// This is allocated on the function's allocator and so lives the life of
789789
/// the function.
790-
MachineInstr::ExtraInfo *
791-
createMIExtraInfo(ArrayRef<MachineMemOperand *> MMOs,
792-
MCSymbol *PreInstrSymbol = nullptr,
793-
MCSymbol *PostInstrSymbol = nullptr);
790+
MachineInstr::ExtraInfo *createMIExtraInfo(
791+
ArrayRef<MachineMemOperand *> MMOs, MCSymbol *PreInstrSymbol = nullptr,
792+
MCSymbol *PostInstrSymbol = nullptr, MDNode *HeapAllocMarker = nullptr);
794793

795794
/// Allocate a string and populate it with the given external symbol name.
796795
const char *createExternalSymbolName(StringRef Name);
@@ -934,14 +933,6 @@ class MachineFunction {
934933
return CodeViewAnnotations;
935934
}
936935

937-
/// Record heapallocsites
938-
void addCodeViewHeapAllocSite(MachineInstr *I, MDNode *MD);
939-
940-
ArrayRef<std::tuple<MCSymbol*, MCSymbol*, DIType*>>
941-
getCodeViewHeapAllocSites() const {
942-
return CodeViewHeapAllocSites;
943-
}
944-
945936
/// Return a reference to the C++ typeinfo for the current function.
946937
const std::vector<const GlobalValue *> &getTypeInfos() const {
947938
return TypeInfos;

‎llvm/include/llvm/CodeGen/MachineInstr.h

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -137,19 +137,23 @@ class MachineInstr
137137
/// This has to be defined eagerly due to the implementation constraints of
138138
/// `PointerSumType` where it is used.
139139
class ExtraInfo final
140-
: TrailingObjects<ExtraInfo, MachineMemOperand *, MCSymbol *> {
140+
: TrailingObjects<ExtraInfo, MachineMemOperand *, MCSymbol *, MDNode *> {
141141
public:
142142
static ExtraInfo *create(BumpPtrAllocator &Allocator,
143143
ArrayRef<MachineMemOperand *> MMOs,
144144
MCSymbol *PreInstrSymbol = nullptr,
145-
MCSymbol *PostInstrSymbol = nullptr) {
145+
MCSymbol *PostInstrSymbol = nullptr,
146+
MDNode *HeapAllocMarker = nullptr) {
146147
bool HasPreInstrSymbol = PreInstrSymbol != nullptr;
147148
bool HasPostInstrSymbol = PostInstrSymbol != nullptr;
149+
bool HasHeapAllocMarker = HeapAllocMarker != nullptr;
148150
auto *Result = new (Allocator.Allocate(
149-
totalSizeToAlloc<MachineMemOperand *, MCSymbol *>(
150-
MMOs.size(), HasPreInstrSymbol + HasPostInstrSymbol),
151+
totalSizeToAlloc<MachineMemOperand *, MCSymbol *, MDNode *>(
152+
MMOs.size(), HasPreInstrSymbol + HasPostInstrSymbol,
153+
HasHeapAllocMarker),
151154
alignof(ExtraInfo)))
152-
ExtraInfo(MMOs.size(), HasPreInstrSymbol, HasPostInstrSymbol);
155+
ExtraInfo(MMOs.size(), HasPreInstrSymbol, HasPostInstrSymbol,
156+
HasHeapAllocMarker);
153157

154158
// Copy the actual data into the trailing objects.
155159
std::copy(MMOs.begin(), MMOs.end(),
@@ -160,6 +164,8 @@ class MachineInstr
160164
if (HasPostInstrSymbol)
161165
Result->getTrailingObjects<MCSymbol *>()[HasPreInstrSymbol] =
162166
PostInstrSymbol;
167+
if (HasHeapAllocMarker)
168+
Result->getTrailingObjects<MDNode *>()[0] = HeapAllocMarker;
163169

164170
return Result;
165171
}
@@ -178,6 +184,10 @@ class MachineInstr
178184
: nullptr;
179185
}
180186

187+
MDNode *getHeapAllocMarker() const {
188+
return HasHeapAllocMarker ? getTrailingObjects<MDNode *>()[0] : nullptr;
189+
}
190+
181191
private:
182192
friend TrailingObjects;
183193

@@ -189,6 +199,7 @@ class MachineInstr
189199
const int NumMMOs;
190200
const bool HasPreInstrSymbol;
191201
const bool HasPostInstrSymbol;
202+
const bool HasHeapAllocMarker;
192203

193204
// Implement the `TrailingObjects` internal API.
194205
size_t numTrailingObjects(OverloadToken<MachineMemOperand *>) const {
@@ -197,12 +208,17 @@ class MachineInstr
197208
size_t numTrailingObjects(OverloadToken<MCSymbol *>) const {
198209
return HasPreInstrSymbol + HasPostInstrSymbol;
199210
}
211+
size_t numTrailingObjects(OverloadToken<MDNode *>) const {
212+
return HasHeapAllocMarker;
213+
}
200214

201215
// Just a boring constructor to allow us to initialize the sizes. Always use
202216
// the `create` routine above.
203-
ExtraInfo(int NumMMOs, bool HasPreInstrSymbol, bool HasPostInstrSymbol)
217+
ExtraInfo(int NumMMOs, bool HasPreInstrSymbol, bool HasPostInstrSymbol,
218+
bool HasHeapAllocMarker)
204219
: NumMMOs(NumMMOs), HasPreInstrSymbol(HasPreInstrSymbol),
205-
HasPostInstrSymbol(HasPostInstrSymbol) {}
220+
HasPostInstrSymbol(HasPostInstrSymbol),
221+
HasHeapAllocMarker(HasHeapAllocMarker) {}
206222
};
207223

208224
/// Enumeration of the kinds of inline extra info available. It is important
@@ -577,6 +593,16 @@ class MachineInstr
577593
return nullptr;
578594
}
579595

596+
/// Helper to extract a heap alloc marker if one has been added.
597+
MDNode *getHeapAllocMarker() const {
598+
if (!Info)
599+
return nullptr;
600+
if (ExtraInfo *EI = Info.get<EIIK_OutOfLine>())
601+
return EI->getHeapAllocMarker();
602+
603+
return nullptr;
604+
}
605+
580606
/// API for querying MachineInstr properties. They are the same as MCInstrDesc
581607
/// queries but they are bundle aware.
582608

@@ -1578,6 +1604,12 @@ class MachineInstr
15781604
/// replace ours with it.
15791605
void cloneInstrSymbols(MachineFunction &MF, const MachineInstr &MI);
15801606

1607+
/// Set a marker on instructions that denotes where we should create and emit
1608+
/// heap alloc site labels. This waits until after instruction selection and
1609+
/// optimizations to create the label, so it should still work if the
1610+
/// instruction is removed or duplicated.
1611+
void setHeapAllocMarker(MachineFunction &MF, MDNode *MD);
1612+
15811613
/// Return the MIFlags which represent both MachineInstrs. This
15821614
/// should be used when merging two MachineInstrs into one. This routine does
15831615
/// not modify the MIFlags of this MachineInstr.
@@ -1632,6 +1664,12 @@ class MachineInstr
16321664
const TargetRegisterClass *getRegClassConstraintEffectForVRegImpl(
16331665
unsigned OpIdx, unsigned Reg, const TargetRegisterClass *CurRC,
16341666
const TargetInstrInfo *TII, const TargetRegisterInfo *TRI) const;
1667+
1668+
/// Stores extra instruction information inline or allocates as ExtraInfo
1669+
/// based on the number of pointers.
1670+
void setExtraInfo(MachineFunction &MF, ArrayRef<MachineMemOperand *> MMOs,
1671+
MCSymbol *PreInstrSymbol, MCSymbol *PostInstrSymbol,
1672+
MDNode *HeapAllocMarker);
16351673
};
16361674

16371675
/// Special DenseMapInfo traits to compare MachineInstr* by *value* of the

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

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1052,13 +1052,9 @@ void AsmPrinter::EmitFunctionBody() {
10521052
++NumInstsInFunction;
10531053
}
10541054

1055-
// If there is a pre-instruction symbol, emit a label for it here. If the
1056-
// instruction was duplicated and the label has already been emitted,
1057-
// don't re-emit the same label.
1058-
// FIXME: Consider strengthening that to an assertion.
1055+
// If there is a pre-instruction symbol, emit a label for it here.
10591056
if (MCSymbol *S = MI.getPreInstrSymbol())
1060-
if (S->isUndefined())
1061-
OutStreamer->EmitLabel(S);
1057+
OutStreamer->EmitLabel(S);
10621058

10631059
if (ShouldPrintDebugScopes) {
10641060
for (const HandlerInfo &HI : Handlers) {
@@ -1111,13 +1107,9 @@ void AsmPrinter::EmitFunctionBody() {
11111107
break;
11121108
}
11131109

1114-
// If there is a post-instruction symbol, emit a label for it here. If
1115-
// the instruction was duplicated and the label has already been emitted,
1116-
// don't re-emit the same label.
1117-
// FIXME: Consider strengthening that to an assertion.
1110+
// If there is a post-instruction symbol, emit a label for it here.
11181111
if (MCSymbol *S = MI.getPostInstrSymbol())
1119-
if (S->isUndefined())
1120-
OutStreamer->EmitLabel(S);
1112+
OutStreamer->EmitLabel(S);
11211113

11221114
if (ShouldPrintDebugScopes) {
11231115
for (const HandlerInfo &HI : Handlers) {

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

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,15 +1127,9 @@ void CodeViewDebug::emitDebugInfoForFunction(const Function *GV,
11271127
}
11281128

11291129
for (auto HeapAllocSite : FI.HeapAllocSites) {
1130-
MCSymbol *BeginLabel = std::get<0>(HeapAllocSite);
1131-
MCSymbol *EndLabel = std::get<1>(HeapAllocSite);
1132-
1133-
// The labels might not be defined if the instruction was replaced
1134-
// somewhere in the codegen pipeline.
1135-
if (!BeginLabel->isDefined() || !EndLabel->isDefined())
1136-
continue;
1137-
1138-
DIType *DITy = std::get<2>(HeapAllocSite);
1130+
const MCSymbol *BeginLabel = std::get<0>(HeapAllocSite);
1131+
const MCSymbol *EndLabel = std::get<1>(HeapAllocSite);
1132+
const DIType *DITy = std::get<2>(HeapAllocSite);
11391133
MCSymbol *HeapAllocEnd = beginSymbolRecord(SymbolKind::S_HEAPALLOCSITE);
11401134
OS.AddComment("Call site offset");
11411135
OS.EmitCOFFSecRel32(BeginLabel, /*Offset=*/0);
@@ -1454,6 +1448,16 @@ void CodeViewDebug::beginFunctionImpl(const MachineFunction *MF) {
14541448
DebugLoc FnStartDL = PrologEndLoc.getFnDebugLoc();
14551449
maybeRecordLocation(FnStartDL, MF);
14561450
}
1451+
1452+
// Find heap alloc sites and emit labels around them.
1453+
for (const auto &MBB : *MF) {
1454+
for (const auto &MI : MBB) {
1455+
if (MI.getHeapAllocMarker()) {
1456+
requestLabelBeforeInsn(&MI);
1457+
requestLabelAfterInsn(&MI);
1458+
}
1459+
}
1460+
}
14571461
}
14581462

14591463
static bool shouldEmitUdt(const DIType *T) {
@@ -2888,8 +2892,18 @@ void CodeViewDebug::endFunctionImpl(const MachineFunction *MF) {
28882892
return;
28892893
}
28902894

2895+
// Find heap alloc sites and add to list.
2896+
for (const auto &MBB : *MF) {
2897+
for (const auto &MI : MBB) {
2898+
if (MDNode *MD = MI.getHeapAllocMarker()) {
2899+
CurFn->HeapAllocSites.push_back(std::make_tuple(getLabelBeforeInsn(&MI),
2900+
getLabelAfterInsn(&MI),
2901+
dyn_cast<DIType>(MD)));
2902+
}
2903+
}
2904+
}
2905+
28912906
CurFn->Annotations = MF->getCodeViewAnnotations();
2892-
CurFn->HeapAllocSites = MF->getCodeViewHeapAllocSites();
28932907

28942908
CurFn->End = Asm->getFunctionEnd();
28952909

‎llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,8 @@ class LLVM_LIBRARY_VISIBILITY CodeViewDebug : public DebugHandlerBase {
148148
SmallVector<LexicalBlock *, 1> ChildBlocks;
149149

150150
std::vector<std::pair<MCSymbol *, MDNode *>> Annotations;
151-
std::vector<std::tuple<MCSymbol *, MCSymbol *, DIType *>> HeapAllocSites;
151+
std::vector<std::tuple<const MCSymbol *, const MCSymbol *, const DIType *>>
152+
HeapAllocSites;
152153

153154
const MCSymbol *Begin = nullptr;
154155
const MCSymbol *End = nullptr;

‎llvm/lib/CodeGen/MachineFunction.cpp

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -446,12 +446,11 @@ MachineFunction::getMachineMemOperand(const MachineMemOperand *MMO,
446446
MMO->getOrdering(), MMO->getFailureOrdering());
447447
}
448448

449-
MachineInstr::ExtraInfo *
450-
MachineFunction::createMIExtraInfo(ArrayRef<MachineMemOperand *> MMOs,
451-
MCSymbol *PreInstrSymbol,
452-
MCSymbol *PostInstrSymbol) {
449+
MachineInstr::ExtraInfo *MachineFunction::createMIExtraInfo(
450+
ArrayRef<MachineMemOperand *> MMOs, MCSymbol *PreInstrSymbol,
451+
MCSymbol *PostInstrSymbol, MDNode *HeapAllocMarker) {
453452
return MachineInstr::ExtraInfo::create(Allocator, MMOs, PreInstrSymbol,
454-
PostInstrSymbol);
453+
PostInstrSymbol, HeapAllocMarker);
455454
}
456455

457456
const char *MachineFunction::createExternalSymbolName(StringRef Name) {
@@ -823,19 +822,12 @@ try_next:;
823822
return FilterID;
824823
}
825824

826-
void MachineFunction::addCodeViewHeapAllocSite(MachineInstr *I, MDNode *MD) {
827-
MCSymbol *BeginLabel = Ctx.createTempSymbol("heapallocsite", true);
828-
MCSymbol *EndLabel = Ctx.createTempSymbol("heapallocsite", true);
829-
I->setPreInstrSymbol(*this, BeginLabel);
830-
I->setPostInstrSymbol(*this, EndLabel);
825+
void MachineFunction::moveCallSiteInfo(const MachineInstr *Old,
826+
const MachineInstr *New) {
827+
assert(New->isCall() && "Call site info refers only to call instructions!");
831828

832-
DIType *DI = dyn_cast<DIType>(MD);
833-
CodeViewHeapAllocSites.push_back(std::make_tuple(BeginLabel, EndLabel, DI));
834-
}
835-
836-
void MachineFunction::updateCallSiteInfo(const MachineInstr *Old,
837-
const MachineInstr *New) {
838-
if (!Target.Options.EnableDebugEntryValues || Old == New)
829+
CallSiteInfoMap::iterator CSIt = getCallSiteInfo(Old);
830+
if (CSIt == CallSitesInfo.end())
839831
return;
840832

841833
assert(Old->isCall() && (!New || New->isCall()) &&

0 commit comments

Comments
 (0)