[llvm] 64c1f66 - Revert "Add an instruction marker field to the ExtraInfo in MachineInstrs."
Amy Huang via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 25 12:42:28 PDT 2019
Author: Amy Huang
Date: 2019-10-25T12:41:34-07:00
New Revision: 64c1f6602a029e3b0914b95d5b580e4b02fc43c1
URL: https://github.com/llvm/llvm-project/commit/64c1f6602a029e3b0914b95d5b580e4b02fc43c1
DIFF: https://github.com/llvm/llvm-project/commit/64c1f6602a029e3b0914b95d5b580e4b02fc43c1.diff
LOG: Revert "Add an instruction marker field to the ExtraInfo in MachineInstrs."
Reverting commit b85b4e5a6f8579c137fecb59a4d75d7bfb111f79 due to some
buildbot failures/ out of memory errors.
Added:
Modified:
llvm/include/llvm/CodeGen/MachineFunction.h
llvm/include/llvm/CodeGen/MachineInstr.h
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
llvm/lib/CodeGen/MachineFunction.cpp
llvm/lib/CodeGen/MachineInstr.cpp
llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
llvm/test/CodeGen/X86/label-heapallocsite.ll
llvm/test/CodeGen/X86/taildup-heapallocsite.ll
llvm/unittests/CodeGen/MachineInstrTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/MachineFunction.h b/llvm/include/llvm/CodeGen/MachineFunction.h
index 681cc8c0dba5..3a3176e51c51 100644
--- a/llvm/include/llvm/CodeGen/MachineFunction.h
+++ b/llvm/include/llvm/CodeGen/MachineFunction.h
@@ -322,6 +322,10 @@ class MachineFunction {
/// CodeView label annotations.
std::vector<std::pair<MCSymbol *, MDNode *>> CodeViewAnnotations;
+ /// CodeView heapallocsites.
+ std::vector<std::tuple<MCSymbol *, MCSymbol *, const DIType *>>
+ CodeViewHeapAllocSites;
+
bool CallsEHReturn = false;
bool CallsUnwindInit = false;
bool HasEHScopes = false;
@@ -796,9 +800,10 @@ class MachineFunction {
///
/// This is allocated on the function's allocator and so lives the life of
/// the function.
- MachineInstr::ExtraInfo *createMIExtraInfo(
- ArrayRef<MachineMemOperand *> MMOs, MCSymbol *PreInstrSymbol = nullptr,
- MCSymbol *PostInstrSymbol = nullptr, MDNode *HeapAllocMarker = nullptr);
+ MachineInstr::ExtraInfo *
+ createMIExtraInfo(ArrayRef<MachineMemOperand *> MMOs,
+ MCSymbol *PreInstrSymbol = nullptr,
+ MCSymbol *PostInstrSymbol = nullptr);
/// Allocate a string and populate it with the given external symbol name.
const char *createExternalSymbolName(StringRef Name);
@@ -942,6 +947,14 @@ class MachineFunction {
return CodeViewAnnotations;
}
+ /// Record heapallocsites
+ void addCodeViewHeapAllocSite(MachineInstr *I, const MDNode *MD);
+
+ ArrayRef<std::tuple<MCSymbol *, MCSymbol *, const DIType *>>
+ getCodeViewHeapAllocSites() const {
+ return CodeViewHeapAllocSites;
+ }
+
/// Return a reference to the C++ typeinfo for the current function.
const std::vector<const GlobalValue *> &getTypeInfos() const {
return TypeInfos;
diff --git a/llvm/include/llvm/CodeGen/MachineInstr.h b/llvm/include/llvm/CodeGen/MachineInstr.h
index 1cd8514696f0..c94ad292ec96 100644
--- a/llvm/include/llvm/CodeGen/MachineInstr.h
+++ b/llvm/include/llvm/CodeGen/MachineInstr.h
@@ -136,23 +136,19 @@ class MachineInstr
/// This has to be defined eagerly due to the implementation constraints of
/// `PointerSumType` where it is used.
class ExtraInfo final
- : TrailingObjects<ExtraInfo, MachineMemOperand *, MCSymbol *, MDNode *> {
+ : TrailingObjects<ExtraInfo, MachineMemOperand *, MCSymbol *> {
public:
static ExtraInfo *create(BumpPtrAllocator &Allocator,
ArrayRef<MachineMemOperand *> MMOs,
MCSymbol *PreInstrSymbol = nullptr,
- MCSymbol *PostInstrSymbol = nullptr,
- MDNode *HeapAllocMarker = nullptr) {
+ MCSymbol *PostInstrSymbol = nullptr) {
bool HasPreInstrSymbol = PreInstrSymbol != nullptr;
bool HasPostInstrSymbol = PostInstrSymbol != nullptr;
- bool HasHeapAllocMarker = HeapAllocMarker != nullptr;
auto *Result = new (Allocator.Allocate(
- totalSizeToAlloc<MachineMemOperand *, MCSymbol *, MDNode *>(
- MMOs.size(), HasPreInstrSymbol + HasPostInstrSymbol,
- HasHeapAllocMarker),
+ totalSizeToAlloc<MachineMemOperand *, MCSymbol *>(
+ MMOs.size(), HasPreInstrSymbol + HasPostInstrSymbol),
alignof(ExtraInfo)))
- ExtraInfo(MMOs.size(), HasPreInstrSymbol, HasPostInstrSymbol,
- HasHeapAllocMarker);
+ ExtraInfo(MMOs.size(), HasPreInstrSymbol, HasPostInstrSymbol);
// Copy the actual data into the trailing objects.
std::copy(MMOs.begin(), MMOs.end(),
@@ -163,10 +159,6 @@ class MachineInstr
if (HasPostInstrSymbol)
Result->getTrailingObjects<MCSymbol *>()[HasPreInstrSymbol] =
PostInstrSymbol;
- if (HasHeapAllocMarker)
- Result->getTrailingObjects<MDNode *>()[HasPreInstrSymbol +
- HasPostInstrSymbol] =
- HeapAllocMarker;
return Result;
}
@@ -185,13 +177,6 @@ class MachineInstr
: nullptr;
}
- MDNode *getHeapAllocMarker() const {
- return HasHeapAllocMarker
- ? getTrailingObjects<MDNode *>()[HasPreInstrSymbol +
- HasPostInstrSymbol]
- : nullptr;
- }
-
private:
friend TrailingObjects;
@@ -203,7 +188,6 @@ class MachineInstr
const int NumMMOs;
const bool HasPreInstrSymbol;
const bool HasPostInstrSymbol;
- const bool HasHeapAllocMarker;
// Implement the `TrailingObjects` internal API.
size_t numTrailingObjects(OverloadToken<MachineMemOperand *>) const {
@@ -212,17 +196,12 @@ class MachineInstr
size_t numTrailingObjects(OverloadToken<MCSymbol *>) const {
return HasPreInstrSymbol + HasPostInstrSymbol;
}
- size_t numTrailingObjects(OverloadToken<MDNode *>) const {
- return HasHeapAllocMarker;
- }
// Just a boring constructor to allow us to initialize the sizes. Always use
// the `create` routine above.
- ExtraInfo(int NumMMOs, bool HasPreInstrSymbol, bool HasPostInstrSymbol,
- bool HasHeapAllocMarker)
+ ExtraInfo(int NumMMOs, bool HasPreInstrSymbol, bool HasPostInstrSymbol)
: NumMMOs(NumMMOs), HasPreInstrSymbol(HasPreInstrSymbol),
- HasPostInstrSymbol(HasPostInstrSymbol),
- HasHeapAllocMarker(HasHeapAllocMarker) {}
+ HasPostInstrSymbol(HasPostInstrSymbol) {}
};
/// Enumeration of the kinds of inline extra info available. It is important
@@ -232,8 +211,7 @@ class MachineInstr
EIIK_MMO = 0,
EIIK_PreInstrSymbol,
EIIK_PostInstrSymbol,
- EIIK_HeapAllocMarker,
- EIIK_OutOfLine,
+ EIIK_OutOfLine
};
// We store extra information about the instruction here. The common case is
@@ -245,7 +223,6 @@ class MachineInstr
PointerSumTypeMember<EIIK_MMO, MachineMemOperand *>,
PointerSumTypeMember<EIIK_PreInstrSymbol, MCSymbol *>,
PointerSumTypeMember<EIIK_PostInstrSymbol, MCSymbol *>,
- PointerSumTypeMember<EIIK_HeapAllocMarker, MDNode *>,
PointerSumTypeMember<EIIK_OutOfLine, ExtraInfo *>>
Info;
@@ -615,17 +592,6 @@ class MachineInstr
return nullptr;
}
- MDNode *getHeapAllocMarker() const {
- if (!Info)
- return nullptr;
- if (MDNode *S = Info.get<EIIK_HeapAllocMarker>())
- return S;
- if (ExtraInfo *EI = Info.get<EIIK_OutOfLine>())
- return EI->getHeapAllocMarker();
-
- return nullptr;
- }
-
/// API for querying MachineInstr properties. They are the same as MCInstrDesc
/// queries but they are bundle aware.
@@ -1631,12 +1597,6 @@ class MachineInstr
/// replace ours with it.
void cloneInstrSymbols(MachineFunction &MF, const MachineInstr &MI);
- /// Set a marker on instructions that denotes where we should create and emit
- /// heap alloc site labels. This waits until after instruction selection and
- /// optimizations to create the label, so it should still work if the
- /// instruction is removed or duplicated.
- void setHeapAllocMarker(MachineFunction &MF, MDNode *DI);
-
/// Return the MIFlags which represent both MachineInstrs. This
/// should be used when merging two MachineInstrs into one. This routine does
/// not modify the MIFlags of this MachineInstr.
@@ -1697,12 +1657,6 @@ class MachineInstr
const TargetRegisterClass *getRegClassConstraintEffectForVRegImpl(
unsigned OpIdx, Register Reg, const TargetRegisterClass *CurRC,
const TargetInstrInfo *TII, const TargetRegisterInfo *TRI) const;
-
- /// Stores extra instruction information inline or allocates as ExtraInfo
- /// based on the number of pointers.
- void setExtraInfo(MachineFunction &MF, ArrayRef<MachineMemOperand *> MMOs,
- MCSymbol *PreInstrSymbol, MCSymbol *PostInstrSymbol,
- MDNode *HeapAllocMarker);
};
/// Special DenseMapInfo traits to compare MachineInstr* by *value* of the
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 513361e13417..73c53d6c4af5 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -1065,9 +1065,13 @@ void AsmPrinter::EmitFunctionBody() {
++NumInstsInFunction;
}
- // If there is a pre-instruction symbol, emit a label for it here.
+ // If there is a pre-instruction symbol, emit a label for it here. If the
+ // instruction was duplicated and the label has already been emitted,
+ // don't re-emit the same label.
+ // FIXME: Consider strengthening that to an assertion.
if (MCSymbol *S = MI.getPreInstrSymbol())
- OutStreamer->EmitLabel(S);
+ if (S->isUndefined())
+ OutStreamer->EmitLabel(S);
if (ShouldPrintDebugScopes) {
for (const HandlerInfo &HI : Handlers) {
@@ -1120,9 +1124,13 @@ void AsmPrinter::EmitFunctionBody() {
break;
}
- // If there is a post-instruction symbol, emit a label for it here.
+ // If there is a post-instruction symbol, emit a label for it here. If
+ // the instruction was duplicated and the label has already been emitted,
+ // don't re-emit the same label.
+ // FIXME: Consider strengthening that to an assertion.
if (MCSymbol *S = MI.getPostInstrSymbol())
- OutStreamer->EmitLabel(S);
+ if (S->isUndefined())
+ OutStreamer->EmitLabel(S);
if (ShouldPrintDebugScopes) {
for (const HandlerInfo &HI : Handlers) {
diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
index 62ad356e7f8f..c6457f3626d1 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -1100,8 +1100,14 @@ void CodeViewDebug::emitDebugInfoForFunction(const Function *GV,
}
for (auto HeapAllocSite : FI.HeapAllocSites) {
- const MCSymbol *BeginLabel = std::get<0>(HeapAllocSite);
- const MCSymbol *EndLabel = std::get<1>(HeapAllocSite);
+ MCSymbol *BeginLabel = std::get<0>(HeapAllocSite);
+ MCSymbol *EndLabel = std::get<1>(HeapAllocSite);
+
+ // The labels might not be defined if the instruction was replaced
+ // somewhere in the codegen pipeline.
+ if (!BeginLabel->isDefined() || !EndLabel->isDefined())
+ continue;
+
const DIType *DITy = std::get<2>(HeapAllocSite);
MCSymbol *HeapAllocEnd = beginSymbolRecord(SymbolKind::S_HEAPALLOCSITE);
OS.AddComment("Call site offset");
@@ -1421,16 +1427,6 @@ void CodeViewDebug::beginFunctionImpl(const MachineFunction *MF) {
DebugLoc FnStartDL = PrologEndLoc.getFnDebugLoc();
maybeRecordLocation(FnStartDL, MF);
}
-
- // Find heap alloc sites and emit labels around them.
- for (const auto &MBB : *MF) {
- for (const auto &MI : MBB) {
- if (MI.getHeapAllocMarker()) {
- requestLabelBeforeInsn(&MI);
- requestLabelAfterInsn(&MI);
- }
- }
- }
}
static bool shouldEmitUdt(const DIType *T) {
@@ -2854,18 +2850,8 @@ void CodeViewDebug::endFunctionImpl(const MachineFunction *MF) {
return;
}
- // Find heap alloc sites and add to list.
- for (const auto &MBB : *MF) {
- for (const auto &MI : MBB) {
- if (MDNode *MD = MI.getHeapAllocMarker()) {
- CurFn->HeapAllocSites.push_back(std::make_tuple(getLabelBeforeInsn(&MI),
- getLabelAfterInsn(&MI),
- dyn_cast<DIType>(MD)));
- }
- }
- }
-
CurFn->Annotations = MF->getCodeViewAnnotations();
+ CurFn->HeapAllocSites = MF->getCodeViewHeapAllocSites();
CurFn->End = Asm->getFunctionEnd();
diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
index b56b9047e1a9..7ffd77926cf7 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
@@ -148,7 +148,7 @@ class LLVM_LIBRARY_VISIBILITY CodeViewDebug : public DebugHandlerBase {
SmallVector<LexicalBlock *, 1> ChildBlocks;
std::vector<std::pair<MCSymbol *, MDNode *>> Annotations;
- std::vector<std::tuple<const MCSymbol *, const MCSymbol *, const DIType *>>
+ std::vector<std::tuple<MCSymbol *, MCSymbol *, const DIType *>>
HeapAllocSites;
const MCSymbol *Begin = nullptr;
diff --git a/llvm/lib/CodeGen/MachineFunction.cpp b/llvm/lib/CodeGen/MachineFunction.cpp
index d75e77d46134..7d2ee230ca9f 100644
--- a/llvm/lib/CodeGen/MachineFunction.cpp
+++ b/llvm/lib/CodeGen/MachineFunction.cpp
@@ -447,11 +447,12 @@ MachineFunction::getMachineMemOperand(const MachineMemOperand *MMO,
MMO->getOrdering(), MMO->getFailureOrdering());
}
-MachineInstr::ExtraInfo *MachineFunction::createMIExtraInfo(
- ArrayRef<MachineMemOperand *> MMOs, MCSymbol *PreInstrSymbol,
- MCSymbol *PostInstrSymbol, MDNode *HeapAllocMarker) {
+MachineInstr::ExtraInfo *
+MachineFunction::createMIExtraInfo(ArrayRef<MachineMemOperand *> MMOs,
+ MCSymbol *PreInstrSymbol,
+ MCSymbol *PostInstrSymbol) {
return MachineInstr::ExtraInfo::create(Allocator, MMOs, PreInstrSymbol,
- PostInstrSymbol, HeapAllocMarker);
+ PostInstrSymbol);
}
const char *MachineFunction::createExternalSymbolName(StringRef Name) {
@@ -823,6 +824,17 @@ try_next:;
return FilterID;
}
+void MachineFunction::addCodeViewHeapAllocSite(MachineInstr *I,
+ const MDNode *MD) {
+ MCSymbol *BeginLabel = Ctx.createTempSymbol("heapallocsite", true);
+ MCSymbol *EndLabel = Ctx.createTempSymbol("heapallocsite", true);
+ I->setPreInstrSymbol(*this, BeginLabel);
+ I->setPostInstrSymbol(*this, EndLabel);
+
+ const DIType *DI = dyn_cast<DIType>(MD);
+ CodeViewHeapAllocSites.push_back(std::make_tuple(BeginLabel, EndLabel, DI));
+}
+
void MachineFunction::moveCallSiteInfo(const MachineInstr *Old,
const MachineInstr *New) {
assert(New->isCall() && "Call site info refers only to call instructions!");
diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp
index 9c717d242131..fec20b2b1a05 100644
--- a/llvm/lib/CodeGen/MachineInstr.cpp
+++ b/llvm/lib/CodeGen/MachineInstr.cpp
@@ -316,48 +316,27 @@ void MachineInstr::RemoveOperand(unsigned OpNo) {
--NumOperands;
}
-void MachineInstr::setExtraInfo(MachineFunction &MF,
- ArrayRef<MachineMemOperand *> MMOs,
- MCSymbol *PreInstrSymbol,
- MCSymbol *PostInstrSymbol,
- MDNode *HeapAllocMarker) {
- bool HasPreInstrSymbol = PreInstrSymbol != nullptr;
- bool HasPostInstrSymbol = PostInstrSymbol != nullptr;
- bool HasHeapAllocMarker = HeapAllocMarker != nullptr;
- int NumPointers =
- MMOs.size() + HasPreInstrSymbol + HasPostInstrSymbol + HasHeapAllocMarker;
-
- // Drop all extra info if there is none.
- if (NumPointers <= 0) {
+void MachineInstr::dropMemRefs(MachineFunction &MF) {
+ if (memoperands_empty())
+ return;
+
+ // See if we can just drop all of our extra info.
+ if (!getPreInstrSymbol() && !getPostInstrSymbol()) {
Info.clear();
return;
}
-
- // If more than one pointer, then store out of line.
- // FIXME: Maybe we should make the symbols in the extra info mutable?
- else if (NumPointers > 1) {
- Info.set<EIIK_OutOfLine>(MF.createMIExtraInfo(
- MMOs, PreInstrSymbol, PostInstrSymbol, HeapAllocMarker));
+ if (!getPostInstrSymbol()) {
+ Info.set<EIIK_PreInstrSymbol>(getPreInstrSymbol());
return;
}
-
- // Otherwise store the single pointer inline.
- if (HasPreInstrSymbol)
- Info.set<EIIK_PreInstrSymbol>(PreInstrSymbol);
- else if (HasPostInstrSymbol)
- Info.set<EIIK_PostInstrSymbol>(PostInstrSymbol);
- else if (HasHeapAllocMarker)
- Info.set<EIIK_HeapAllocMarker>(HeapAllocMarker);
- else
- Info.set<EIIK_MMO>(MMOs[0]);
-}
-
-void MachineInstr::dropMemRefs(MachineFunction &MF) {
- if (memoperands_empty())
+ if (!getPreInstrSymbol()) {
+ Info.set<EIIK_PostInstrSymbol>(getPostInstrSymbol());
return;
+ }
- setExtraInfo(MF, {}, getPreInstrSymbol(), getPostInstrSymbol(),
- getHeapAllocMarker());
+ // Otherwise allocate a fresh extra info with just these symbols.
+ Info.set<EIIK_OutOfLine>(
+ MF.createMIExtraInfo({}, getPreInstrSymbol(), getPostInstrSymbol()));
}
void MachineInstr::setMemRefs(MachineFunction &MF,
@@ -367,8 +346,15 @@ void MachineInstr::setMemRefs(MachineFunction &MF,
return;
}
- setExtraInfo(MF, MMOs, getPreInstrSymbol(), getPostInstrSymbol(),
- getHeapAllocMarker());
+ // Try to store a single MMO inline.
+ if (MMOs.size() == 1 && !getPreInstrSymbol() && !getPostInstrSymbol()) {
+ Info.set<EIIK_MMO>(MMOs[0]);
+ return;
+ }
+
+ // Otherwise create an extra info struct with all of our info.
+ Info.set<EIIK_OutOfLine>(
+ MF.createMIExtraInfo(MMOs, getPreInstrSymbol(), getPostInstrSymbol()));
}
void MachineInstr::addMemOperand(MachineFunction &MF,
@@ -390,8 +376,7 @@ void MachineInstr::cloneMemRefs(MachineFunction &MF, const MachineInstr &MI) {
// instruction. We can do this whenever the pre- and post-instruction symbols
// are the same (including null).
if (getPreInstrSymbol() == MI.getPreInstrSymbol() &&
- getPostInstrSymbol() == MI.getPostInstrSymbol() &&
- getHeapAllocMarker() == MI.getHeapAllocMarker()) {
+ getPostInstrSymbol() == MI.getPostInstrSymbol()) {
Info = MI.Info;
return;
}
@@ -465,48 +450,67 @@ void MachineInstr::cloneMergedMemRefs(MachineFunction &MF,
}
void MachineInstr::setPreInstrSymbol(MachineFunction &MF, MCSymbol *Symbol) {
- // Do nothing if old and new symbols are the same.
- if (Symbol == getPreInstrSymbol())
+ MCSymbol *OldSymbol = getPreInstrSymbol();
+ if (OldSymbol == Symbol)
return;
+ if (OldSymbol && !Symbol) {
+ // We're removing a symbol rather than adding one. Try to clean up any
+ // extra info carried around.
+ if (Info.is<EIIK_PreInstrSymbol>()) {
+ Info.clear();
+ return;
+ }
- // If there was only one symbol and we're removing it, just clear info.
- if (!Symbol && Info.is<EIIK_PreInstrSymbol>()) {
- Info.clear();
+ if (memoperands_empty()) {
+ assert(getPostInstrSymbol() &&
+ "Should never have only a single symbol allocated out-of-line!");
+ Info.set<EIIK_PostInstrSymbol>(getPostInstrSymbol());
+ return;
+ }
+
+ // Otherwise fallback on the generic update.
+ } else if (!Info || Info.is<EIIK_PreInstrSymbol>()) {
+ // If we don't have any other extra info, we can store this inline.
+ Info.set<EIIK_PreInstrSymbol>(Symbol);
return;
}
- setExtraInfo(MF, memoperands(), Symbol, getPostInstrSymbol(),
- getHeapAllocMarker());
+ // Otherwise, allocate a full new set of extra info.
+ // FIXME: Maybe we should make the symbols in the extra info mutable?
+ Info.set<EIIK_OutOfLine>(
+ MF.createMIExtraInfo(memoperands(), Symbol, getPostInstrSymbol()));
}
void MachineInstr::setPostInstrSymbol(MachineFunction &MF, MCSymbol *Symbol) {
- // Do nothing if old and new symbols are the same.
- if (Symbol == getPostInstrSymbol())
- return;
-
- // If there was only one symbol and we're removing it, just clear info.
- if (!Symbol && Info.is<EIIK_PostInstrSymbol>()) {
- Info.clear();
+ MCSymbol *OldSymbol = getPostInstrSymbol();
+ if (OldSymbol == Symbol)
return;
- }
-
- setExtraInfo(MF, memoperands(), getPreInstrSymbol(), Symbol,
- getHeapAllocMarker());
-}
+ if (OldSymbol && !Symbol) {
+ // We're removing a symbol rather than adding one. Try to clean up any
+ // extra info carried around.
+ if (Info.is<EIIK_PostInstrSymbol>()) {
+ Info.clear();
+ return;
+ }
-void MachineInstr::setHeapAllocMarker(MachineFunction &MF, MDNode *Marker) {
- // Do nothing if old and new symbols are the same.
- if (Marker == getHeapAllocMarker())
- return;
+ if (memoperands_empty()) {
+ assert(getPreInstrSymbol() &&
+ "Should never have only a single symbol allocated out-of-line!");
+ Info.set<EIIK_PreInstrSymbol>(getPreInstrSymbol());
+ return;
+ }
- // If there was only one symbol and we're removing it, just clear info.
- if (!Marker && Info.is<EIIK_HeapAllocMarker>()) {
- Info.clear();
+ // Otherwise fallback on the generic update.
+ } else if (!Info || Info.is<EIIK_PostInstrSymbol>()) {
+ // If we don't have any other extra info, we can store this inline.
+ Info.set<EIIK_PostInstrSymbol>(Symbol);
return;
}
- setExtraInfo(MF, memoperands(), getPreInstrSymbol(), getPostInstrSymbol(),
- Marker);
+ // Otherwise, allocate a full new set of extra info.
+ // FIXME: Maybe we should make the symbols in the extra info mutable?
+ Info.set<EIIK_OutOfLine>(
+ MF.createMIExtraInfo(memoperands(), getPreInstrSymbol(), Symbol));
}
void MachineInstr::cloneInstrSymbols(MachineFunction &MF,
@@ -520,7 +524,6 @@ void MachineInstr::cloneInstrSymbols(MachineFunction &MF,
setPreInstrSymbol(MF, MI.getPreInstrSymbol());
setPostInstrSymbol(MF, MI.getPostInstrSymbol());
- setHeapAllocMarker(MF, MI.getHeapAllocMarker());
}
uint16_t MachineInstr::mergeFlagsWith(const MachineInstr &Other) const {
@@ -1707,13 +1710,6 @@ void MachineInstr::print(raw_ostream &OS, ModuleSlotTracker &MST,
OS << " post-instr-symbol ";
MachineOperand::printSymbol(OS, *PostInstrSymbol);
}
- if (MDNode *HeapAllocMarker = getHeapAllocMarker()) {
- if (!FirstOp) {
- FirstOp = false;
- OS << ',';
- }
- OS << " heap-alloc-marker";
- }
if (!SkipDebugLoc) {
if (const DebugLoc &DL = getDebugLoc()) {
diff --git a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
index c8304f875f7e..6d7260d7aee5 100644
--- a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
@@ -1236,9 +1236,10 @@ bool FastISel::lowerCallTo(CallLoweringInfo &CLI) {
updateValueMap(CLI.CS->getInstruction(), CLI.ResultReg, CLI.NumResultRegs);
// Set labels for heapallocsite call.
- if (CLI.CS)
- if (MDNode *MD = CLI.CS->getInstruction()->getMetadata("heapallocsite"))
- CLI.Call->setHeapAllocMarker(*MF, MD);
+ if (CLI.CS && CLI.CS->getInstruction()->hasMetadata("heapallocsite")) {
+ const MDNode *MD = CLI.CS->getInstruction()->getMetadata("heapallocsite");
+ MF->addCodeViewHeapAllocSite(CLI.Call, MD);
+ }
return true;
}
diff --git a/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp b/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
index 0e4d783e3505..d4c1fb36475e 100644
--- a/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
@@ -910,9 +910,10 @@ EmitSchedule(MachineBasicBlock::iterator &InsertPos) {
if (HasDbg)
ProcessSourceNode(N, DAG, Emitter, VRBaseMap, Orders, Seen, NewInsn);
- if (MDNode *MD = DAG->getHeapAllocSite(N))
+ if (MDNode *MD = DAG->getHeapAllocSite(N)) {
if (NewInsn && NewInsn->isCall())
- NewInsn->setHeapAllocMarker(MF, MD);
+ MF.addCodeViewHeapAllocSite(NewInsn, MD);
+ }
GluedNodes.pop_back();
}
@@ -922,10 +923,9 @@ EmitSchedule(MachineBasicBlock::iterator &InsertPos) {
if (HasDbg)
ProcessSourceNode(SU->getNode(), DAG, Emitter, VRBaseMap, Orders, Seen,
NewInsn);
-
if (MDNode *MD = DAG->getHeapAllocSite(SU->getNode())) {
if (NewInsn && NewInsn->isCall())
- NewInsn->setHeapAllocMarker(MF, MD);
+ MF.addCodeViewHeapAllocSite(NewInsn, MD);
}
}
diff --git a/llvm/test/CodeGen/X86/label-heapallocsite.ll b/llvm/test/CodeGen/X86/label-heapallocsite.ll
index e76e097d586d..deb74c2ea237 100644
--- a/llvm/test/CodeGen/X86/label-heapallocsite.ll
+++ b/llvm/test/CodeGen/X86/label-heapallocsite.ll
@@ -1,5 +1,5 @@
-; RUN: llc < %s | FileCheck --check-prefixes=CHECK %s
-; RUN: llc -O0 < %s | FileCheck --check-prefixes=CHECK %s
+; RUN: llc < %s | FileCheck --check-prefixes=DAG,CHECK %s
+; RUN: llc -O0 < %s | FileCheck --check-prefixes=FAST,CHECK %s
; Source to regenerate:
; $ clang -cc1 -triple x86_64-windows-msvc t.cpp -debug-info-kind=limited \
@@ -71,33 +71,42 @@ declare void @llvm.dbg.value(metadata, metadata, metadata) #2
; Don't emit metadata for tail calls.
; CHECK-LABEL: call_tail: # @call_tail
+; CHECK-NOT: .Lheapallocsite
; CHECK: jmp alloc_foo
; CHECK-LABEL: call_virtual: # @call_virtual
-; CHECK: callq *{{.*}}%rax{{.*}}
-; CHECK-NEXT: [[LABEL1:.Ltmp[0-9]+]]:
+; CHECK: .Lheapallocsite0:
+; CHECK: callq *{{.*}}%rax{{.*}}
+; CHECK: .Lheapallocsite1:
; CHECK-LABEL: call_multiple: # @call_multiple
-; CHECK: callq alloc_foo
-; CHECK-NEXT: [[LABEL3:.Ltmp[0-9]+]]:
-; CHECK: callq alloc_foo
-; CHECK-NEXT: [[LABEL5:.Ltmp[0-9]+]]:
+; FastISel emits instructions in a
diff erent order.
+; DAG: .Lheapallocsite2:
+; FAST: .Lheapallocsite4:
+; CHECK: callq alloc_foo
+; DAG: .Lheapallocsite3:
+; FAST: .Lheapallocsite5:
+; DAG: .Lheapallocsite4:
+; FAST: .Lheapallocsite2:
+; CHECK: callq alloc_foo
+; DAG: .Lheapallocsite5:
+; FAST: .Lheapallocsite3:
; CHECK-LABEL: .short 4423 # Record kind: S_GPROC32_ID
; CHECK: .short 4446 # Record kind: S_HEAPALLOCSITE
-; CHECK-NEXT: .secrel32 [[LABEL0:.Ltmp[0-9]+]]
-; CHECK-NEXT: .secidx [[LABEL0]]
-; CHECK-NEXT: .short [[LABEL1]]-[[LABEL0]]
+; CHECK-NEXT: .secrel32 .Lheapallocsite0
+; CHECK-NEXT: .secidx .Lheapallocsite0
+; CHECK-NEXT: .short .Lheapallocsite1-.Lheapallocsite0
; CHECK-NEXT: .long 3
; CHECK: .short 4446 # Record kind: S_HEAPALLOCSITE
-; CHECK-NEXT: .secrel32 [[LABEL2:.Ltmp[0-9]+]]
-; CHECK-NEXT: .secidx [[LABEL2]]
-; CHECK-NEXT: .short [[LABEL3]]-[[LABEL2]]
+; CHECK-NEXT: .secrel32 .Lheapallocsite2
+; CHECK-NEXT: .secidx .Lheapallocsite2
+; CHECK-NEXT: .short .Lheapallocsite3-.Lheapallocsite2
; CHECK-NEXT: .long 4096
; CHECK: .short 4446 # Record kind: S_HEAPALLOCSITE
-; CHECK-NEXT: .secrel32 [[LABEL4:.Ltmp[0-9]+]]
-; CHECK-NEXT: .secidx [[LABEL4]]
-; CHECK-NEXT: .short [[LABEL5]]-[[LABEL4]]
+; CHECK-NEXT: .secrel32 .Lheapallocsite4
+; CHECK-NEXT: .secidx .Lheapallocsite4
+; CHECK-NEXT: .short .Lheapallocsite5-.Lheapallocsite4
; CHECK-NEXT: .long 4096
attributes #0 = { nounwind "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="none" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
diff --git a/llvm/test/CodeGen/X86/taildup-heapallocsite.ll b/llvm/test/CodeGen/X86/taildup-heapallocsite.ll
index 281fc9efbc3b..378efe2deec2 100644
--- a/llvm/test/CodeGen/X86/taildup-heapallocsite.ll
+++ b/llvm/test/CodeGen/X86/taildup-heapallocsite.ll
@@ -8,7 +8,8 @@
; f2();
; }
-; In this case, block placement duplicates the heap allocation site.
+; In this case, block placement duplicates the heap allocation site. For now,
+; LLVM drops the labels from one call site. Eventually, we should track both.
; ModuleID = 't.cpp'
source_filename = "t.cpp"
@@ -36,25 +37,15 @@ cond.end: ; preds = %entry, %cond.true
; CHECK-LABEL: taildupit: # @taildupit
; CHECK: testq
; CHECK: je
+; CHECK: .Lheapallocsite0:
; CHECK: callq alloc
-; CHECK-NEXT: [[L1:.Ltmp[0-9]+]]
+; CHECK: .Lheapallocsite1:
; CHECK: jmp f2 # TAILCALL
+; CHECK-NOT: Lheapallocsite
; CHECK: callq alloc
-; CHECK-NEXT: [[L3:.Ltmp[0-9]+]]
+; CHECK-NOT: Lheapallocsite
; CHECK: jmp f2 # TAILCALL
-; CHECK-LABEL: .short 4423 # Record kind: S_GPROC32_ID
-; CHECK: .short 4446 # Record kind: S_HEAPALLOCSITE
-; CHECK-NEXT: .secrel32 [[L0:.Ltmp[0-9]+]]
-; CHECK-NEXT: .secidx [[L0]]
-; CHECK-NEXT: .short [[L1]]-[[L0]]
-; CHECK-NEXT: .long 3
-; CHECK: .short 4446 # Record kind: S_HEAPALLOCSITE
-; CHECK-NEXT: .secrel32 [[L2:.Ltmp[0-9]+]]
-; CHECK-NEXT: .secidx [[L2]]
-; CHECK-NEXT: .short [[L3]]-[[L2]]
-; CHECK-NEXT: .long 3
-
declare dso_local i8* @alloc(i32)
declare dso_local void @f2()
diff --git a/llvm/unittests/CodeGen/MachineInstrTest.cpp b/llvm/unittests/CodeGen/MachineInstrTest.cpp
index 4913c4fd6cdc..d48297734e17 100644
--- a/llvm/unittests/CodeGen/MachineInstrTest.cpp
+++ b/llvm/unittests/CodeGen/MachineInstrTest.cpp
@@ -9,7 +9,6 @@
#include "llvm/CodeGen/MachineBasicBlock.h"
#include "llvm/CodeGen/MachineInstr.h"
#include "llvm/CodeGen/MachineFunction.h"
-#include "llvm/CodeGen/MachineMemOperand.h"
#include "llvm/CodeGen/MachineModuleInfo.h"
#include "llvm/CodeGen/TargetFrameLowering.h"
#include "llvm/CodeGen/TargetInstrInfo.h"
@@ -17,8 +16,6 @@
#include "llvm/CodeGen/TargetSubtargetInfo.h"
#include "llvm/IR/DebugInfoMetadata.h"
#include "llvm/IR/ModuleSlotTracker.h"
-#include "llvm/MC/MCAsmInfo.h"
-#include "llvm/MC/MCSymbol.h"
#include "llvm/Support/TargetRegistry.h"
#include "llvm/Support/TargetSelect.h"
#include "llvm/Target/TargetMachine.h"
@@ -128,7 +125,6 @@ class BogusTargetMachine : public LLVMTargetMachine {
: LLVMTargetMachine(Target(), "", Triple(""), "", "", TargetOptions(),
Reloc::Static, CodeModel::Small, CodeGenOpt::Default),
ST(*this) {}
-
~BogusTargetMachine() override {}
const TargetSubtargetInfo *getSubtargetImpl(const Function &) const override {
@@ -139,17 +135,6 @@ class BogusTargetMachine : public LLVMTargetMachine {
BogusSubtarget ST;
};
-class BogusMCContext : public MCContext {
-public:
- BogusMCContext(MCAsmInfo *mai)
- : MCContext(mai, nullptr, nullptr, nullptr, nullptr, false) {}
-};
-
-std::unique_ptr<BogusMCContext> createMCContext() {
- MCAsmInfo mai = MCAsmInfo();
- return std::make_unique<BogusMCContext>(&mai);
-}
-
std::unique_ptr<BogusTargetMachine> createTargetMachine() {
return std::make_unique<BogusTargetMachine>();
}
@@ -376,135 +361,6 @@ TEST(MachineInstrSpan, DistanceEnd) {
ASSERT_TRUE(std::distance(MIS.begin(), MII) == 1);
}
-TEST(MachineInstrExtraInfo, AddExtraInfo) {
- auto MF = createMachineFunction();
- MCInstrDesc MCID = {0, 0, 0, 0, 0, 0,
- 0, nullptr, nullptr, nullptr, 0, nullptr};
-
- auto MI = MF->CreateMachineInstr(MCID, DebugLoc());
- auto MC = createMCContext();
- auto MMO = MF->getMachineMemOperand(MachinePointerInfo(),
- MachineMemOperand::MOLoad, 8, 8);
- SmallVector<MachineMemOperand *, 2> MMOs;
- MMOs.push_back(MMO);
- MCSymbol *Sym1 = MC->createTempSymbol("pre_label", false);
- MCSymbol *Sym2 = MC->createTempSymbol("post_label", false);
- LLVMContext Ctx;
- MDNode *MDN = MDNode::getDistinct(Ctx, None);
-
- ASSERT_TRUE(MI->memoperands_empty());
- ASSERT_FALSE(MI->getPreInstrSymbol());
- ASSERT_FALSE(MI->getPostInstrSymbol());
- ASSERT_FALSE(MI->getHeapAllocMarker());
-
- MI->setMemRefs(*MF, MMOs);
- ASSERT_TRUE(MI->memoperands().size() == 1);
- ASSERT_FALSE(MI->getPreInstrSymbol());
- ASSERT_FALSE(MI->getPostInstrSymbol());
- ASSERT_FALSE(MI->getHeapAllocMarker());
-
- MI->setPreInstrSymbol(*MF, Sym1);
- ASSERT_TRUE(MI->memoperands().size() == 1);
- ASSERT_TRUE(MI->getPreInstrSymbol() == Sym1);
- ASSERT_FALSE(MI->getPostInstrSymbol());
- ASSERT_FALSE(MI->getHeapAllocMarker());
-
- MI->setPostInstrSymbol(*MF, Sym2);
- ASSERT_TRUE(MI->memoperands().size() == 1);
- ASSERT_TRUE(MI->getPreInstrSymbol() == Sym1);
- ASSERT_TRUE(MI->getPostInstrSymbol() == Sym2);
- ASSERT_FALSE(MI->getHeapAllocMarker());
-
- MI->setHeapAllocMarker(*MF, MDN);
- ASSERT_TRUE(MI->memoperands().size() == 1);
- ASSERT_TRUE(MI->getPreInstrSymbol() == Sym1);
- ASSERT_TRUE(MI->getPostInstrSymbol() == Sym2);
- ASSERT_TRUE(MI->getHeapAllocMarker() == MDN);
-}
-
-TEST(MachineInstrExtraInfo, ChangeExtraInfo) {
- auto MF = createMachineFunction();
- MCInstrDesc MCID = {0, 0, 0, 0, 0, 0,
- 0, nullptr, nullptr, nullptr, 0, nullptr};
-
- auto MI = MF->CreateMachineInstr(MCID, DebugLoc());
- auto MC = createMCContext();
- auto MMO = MF->getMachineMemOperand(MachinePointerInfo(),
- MachineMemOperand::MOLoad, 8, 8);
- SmallVector<MachineMemOperand *, 2> MMOs;
- MMOs.push_back(MMO);
- MCSymbol *Sym1 = MC->createTempSymbol("pre_label", false);
- MCSymbol *Sym2 = MC->createTempSymbol("post_label", false);
- LLVMContext Ctx;
- MDNode *MDN = MDNode::getDistinct(Ctx, None);
-
- MI->setMemRefs(*MF, MMOs);
- MI->setPreInstrSymbol(*MF, Sym1);
- MI->setPostInstrSymbol(*MF, Sym2);
- MI->setHeapAllocMarker(*MF, MDN);
-
- MMOs.push_back(MMO);
-
- MI->setMemRefs(*MF, MMOs);
- ASSERT_TRUE(MI->memoperands().size() == 2);
- ASSERT_TRUE(MI->getPreInstrSymbol() == Sym1);
- ASSERT_TRUE(MI->getPostInstrSymbol() == Sym2);
- ASSERT_TRUE(MI->getHeapAllocMarker() == MDN);
-
- MI->setPostInstrSymbol(*MF, Sym1);
- ASSERT_TRUE(MI->memoperands().size() == 2);
- ASSERT_TRUE(MI->getPreInstrSymbol() == Sym1);
- ASSERT_TRUE(MI->getPostInstrSymbol() == Sym1);
- ASSERT_TRUE(MI->getHeapAllocMarker() == MDN);
-}
-
-TEST(MachineInstrExtraInfo, RemoveExtraInfo) {
- auto MF = createMachineFunction();
- MCInstrDesc MCID = {0, 0, 0, 0, 0, 0,
- 0, nullptr, nullptr, nullptr, 0, nullptr};
-
- auto MI = MF->CreateMachineInstr(MCID, DebugLoc());
- auto MC = createMCContext();
- auto MMO = MF->getMachineMemOperand(MachinePointerInfo(),
- MachineMemOperand::MOLoad, 8, 8);
- SmallVector<MachineMemOperand *, 2> MMOs;
- MMOs.push_back(MMO);
- MMOs.push_back(MMO);
- MCSymbol *Sym1 = MC->createTempSymbol("pre_label", false);
- MCSymbol *Sym2 = MC->createTempSymbol("post_label", false);
- LLVMContext Ctx;
- MDNode *MDN = MDNode::getDistinct(Ctx, None);
-
- MI->setMemRefs(*MF, MMOs);
- MI->setPreInstrSymbol(*MF, Sym1);
- MI->setPostInstrSymbol(*MF, Sym2);
- MI->setHeapAllocMarker(*MF, MDN);
-
- MI->setPostInstrSymbol(*MF, nullptr);
- ASSERT_TRUE(MI->memoperands().size() == 2);
- ASSERT_TRUE(MI->getPreInstrSymbol() == Sym1);
- ASSERT_FALSE(MI->getPostInstrSymbol());
- ASSERT_TRUE(MI->getHeapAllocMarker() == MDN);
-
- MI->setHeapAllocMarker(*MF, nullptr);
- ASSERT_TRUE(MI->memoperands().size() == 2);
- ASSERT_TRUE(MI->getPreInstrSymbol() == Sym1);
- ASSERT_FALSE(MI->getPostInstrSymbol());
- ASSERT_FALSE(MI->getHeapAllocMarker());
-
- MI->setPreInstrSymbol(*MF, nullptr);
- ASSERT_TRUE(MI->memoperands().size() == 2);
- ASSERT_FALSE(MI->getPreInstrSymbol());
- ASSERT_FALSE(MI->getPostInstrSymbol());
- ASSERT_FALSE(MI->getHeapAllocMarker());
-
- MI->setMemRefs(*MF, {});
- ASSERT_TRUE(MI->memoperands_empty());
- ASSERT_FALSE(MI->getPreInstrSymbol());
- ASSERT_FALSE(MI->getPostInstrSymbol());
- ASSERT_FALSE(MI->getHeapAllocMarker());
-}
-
static_assert(is_trivially_copyable<MCOperand>::value, "trivially copyable");
} // end namespace
More information about the llvm-commits
mailing list