diff --git a/llvm/include/llvm/CodeGen/MachineFunction.h b/llvm/include/llvm/CodeGen/MachineFunction.h --- a/llvm/include/llvm/CodeGen/MachineFunction.h +++ b/llvm/include/llvm/CodeGen/MachineFunction.h @@ -326,10 +326,6 @@ /// CodeView label annotations. std::vector> CodeViewAnnotations; - /// CodeView heapallocsites. - std::vector> - CodeViewHeapAllocSites; - bool CallsEHReturn = false; bool CallsUnwindInit = false; bool HasEHScopes = false; @@ -804,10 +800,9 @@ /// /// This is allocated on the function's allocator and so lives the life of /// the function. - MachineInstr::ExtraInfo * - createMIExtraInfo(ArrayRef MMOs, - MCSymbol *PreInstrSymbol = nullptr, - MCSymbol *PostInstrSymbol = nullptr); + MachineInstr::ExtraInfo *createMIExtraInfo( + ArrayRef MMOs, MCSymbol *PreInstrSymbol = nullptr, + MCSymbol *PostInstrSymbol = nullptr, MDNode *HeapAllocMarker = nullptr); /// Allocate a string and populate it with the given external symbol name. const char *createExternalSymbolName(StringRef Name); @@ -962,14 +957,6 @@ return CodeViewAnnotations; } - /// Record heapallocsites - void addCodeViewHeapAllocSite(MachineInstr *I, const MDNode *MD); - - ArrayRef> - getCodeViewHeapAllocSites() const { - return CodeViewHeapAllocSites; - } - /// Return a reference to the C++ typeinfo for the current function. const std::vector &getTypeInfos() const { return TypeInfos; diff --git a/llvm/include/llvm/CodeGen/MachineInstr.h b/llvm/include/llvm/CodeGen/MachineInstr.h --- a/llvm/include/llvm/CodeGen/MachineInstr.h +++ b/llvm/include/llvm/CodeGen/MachineInstr.h @@ -136,19 +136,23 @@ /// This has to be defined eagerly due to the implementation constraints of /// `PointerSumType` where it is used. class ExtraInfo final - : TrailingObjects { + : TrailingObjects { public: static ExtraInfo *create(BumpPtrAllocator &Allocator, ArrayRef MMOs, MCSymbol *PreInstrSymbol = nullptr, - MCSymbol *PostInstrSymbol = nullptr) { + MCSymbol *PostInstrSymbol = nullptr, + MDNode *HeapAllocMarker = nullptr) { bool HasPreInstrSymbol = PreInstrSymbol != nullptr; bool HasPostInstrSymbol = PostInstrSymbol != nullptr; + bool HasHeapAllocMarker = HeapAllocMarker != nullptr; auto *Result = new (Allocator.Allocate( - totalSizeToAlloc( - MMOs.size(), HasPreInstrSymbol + HasPostInstrSymbol), + totalSizeToAlloc( + MMOs.size(), HasPreInstrSymbol + HasPostInstrSymbol, + HasHeapAllocMarker), alignof(ExtraInfo))) - ExtraInfo(MMOs.size(), HasPreInstrSymbol, HasPostInstrSymbol); + ExtraInfo(MMOs.size(), HasPreInstrSymbol, HasPostInstrSymbol, + HasHeapAllocMarker); // Copy the actual data into the trailing objects. std::copy(MMOs.begin(), MMOs.end(), @@ -159,6 +163,8 @@ if (HasPostInstrSymbol) Result->getTrailingObjects()[HasPreInstrSymbol] = PostInstrSymbol; + if (HasHeapAllocMarker) + Result->getTrailingObjects()[0] = HeapAllocMarker; return Result; } @@ -177,6 +183,10 @@ : nullptr; } + MDNode *getHeapAllocMarker() const { + return HasHeapAllocMarker ? getTrailingObjects()[0] : nullptr; + } + private: friend TrailingObjects; @@ -188,6 +198,7 @@ const int NumMMOs; const bool HasPreInstrSymbol; const bool HasPostInstrSymbol; + const bool HasHeapAllocMarker; // Implement the `TrailingObjects` internal API. size_t numTrailingObjects(OverloadToken) const { @@ -196,12 +207,17 @@ size_t numTrailingObjects(OverloadToken) const { return HasPreInstrSymbol + HasPostInstrSymbol; } + size_t numTrailingObjects(OverloadToken) 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) + ExtraInfo(int NumMMOs, bool HasPreInstrSymbol, bool HasPostInstrSymbol, + bool HasHeapAllocMarker) : NumMMOs(NumMMOs), HasPreInstrSymbol(HasPreInstrSymbol), - HasPostInstrSymbol(HasPostInstrSymbol) {} + HasPostInstrSymbol(HasPostInstrSymbol), + HasHeapAllocMarker(HasHeapAllocMarker) {} }; /// Enumeration of the kinds of inline extra info available. It is important @@ -592,6 +608,16 @@ return nullptr; } + /// Helper to extract a heap alloc marker if one has been added. + MDNode *getHeapAllocMarker() const { + if (!Info) + return nullptr; + if (ExtraInfo *EI = Info.get()) + return EI->getHeapAllocMarker(); + + return nullptr; + } + /// API for querying MachineInstr properties. They are the same as MCInstrDesc /// queries but they are bundle aware. @@ -1597,6 +1623,12 @@ /// 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 *MD); + /// 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. @@ -1657,6 +1689,12 @@ 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 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 --- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp @@ -1060,13 +1060,9 @@ ++NumInstsInFunction; } - // 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 there is a pre-instruction symbol, emit a label for it here. if (MCSymbol *S = MI.getPreInstrSymbol()) - if (S->isUndefined()) - OutStreamer->EmitLabel(S); + OutStreamer->EmitLabel(S); if (ShouldPrintDebugScopes) { for (const HandlerInfo &HI : Handlers) { @@ -1119,13 +1115,9 @@ break; } - // 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 there is a post-instruction symbol, emit a label for it here. if (MCSymbol *S = MI.getPostInstrSymbol()) - if (S->isUndefined()) - OutStreamer->EmitLabel(S); + OutStreamer->EmitLabel(S); if (ShouldPrintDebugScopes) { for (const HandlerInfo &HI : Handlers) { diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h --- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h +++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h @@ -148,7 +148,7 @@ SmallVector ChildBlocks; std::vector> Annotations; - std::vector> + std::vector> HeapAllocSites; const MCSymbol *Begin = nullptr; diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp --- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp @@ -1100,14 +1100,8 @@ } for (auto HeapAllocSite : FI.HeapAllocSites) { - 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 MCSymbol *BeginLabel = std::get<0>(HeapAllocSite); + const MCSymbol *EndLabel = std::get<1>(HeapAllocSite); const DIType *DITy = std::get<2>(HeapAllocSite); MCSymbol *HeapAllocEnd = beginSymbolRecord(SymbolKind::S_HEAPALLOCSITE); OS.AddComment("Call site offset"); @@ -1427,6 +1421,16 @@ 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) { @@ -2850,8 +2854,18 @@ 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(MD))); + } + } + } + CurFn->Annotations = MF->getCodeViewAnnotations(); - CurFn->HeapAllocSites = MF->getCodeViewHeapAllocSites(); CurFn->End = Asm->getFunctionEnd(); diff --git a/llvm/lib/CodeGen/MachineFunction.cpp b/llvm/lib/CodeGen/MachineFunction.cpp --- a/llvm/lib/CodeGen/MachineFunction.cpp +++ b/llvm/lib/CodeGen/MachineFunction.cpp @@ -447,12 +447,11 @@ MMO->getOrdering(), MMO->getFailureOrdering()); } -MachineInstr::ExtraInfo * -MachineFunction::createMIExtraInfo(ArrayRef MMOs, - MCSymbol *PreInstrSymbol, - MCSymbol *PostInstrSymbol) { +MachineInstr::ExtraInfo *MachineFunction::createMIExtraInfo( + ArrayRef MMOs, MCSymbol *PreInstrSymbol, + MCSymbol *PostInstrSymbol, MDNode *HeapAllocMarker) { return MachineInstr::ExtraInfo::create(Allocator, MMOs, PreInstrSymbol, - PostInstrSymbol); + PostInstrSymbol, HeapAllocMarker); } const char *MachineFunction::createExternalSymbolName(StringRef Name) { @@ -824,17 +823,6 @@ 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(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 --- a/llvm/lib/CodeGen/MachineInstr.cpp +++ b/llvm/lib/CodeGen/MachineInstr.cpp @@ -316,27 +316,48 @@ --NumOperands; } -void MachineInstr::dropMemRefs(MachineFunction &MF) { - if (memoperands_empty()) - return; - - // See if we can just drop all of our extra info. - if (!getPreInstrSymbol() && !getPostInstrSymbol()) { +void MachineInstr::setExtraInfo(MachineFunction &MF, + ArrayRef 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) { Info.clear(); return; } - if (!getPostInstrSymbol()) { - Info.set(getPreInstrSymbol()); + + // If more than one pointer, then store out of line. Store heap alloc markers + // out of line because PointerSumType cannot hold more than 4 tag types with + // 32-bit pointers. + // FIXME: Maybe we should make the symbols in the extra info mutable? + else if (NumPointers > 1 || HasHeapAllocMarker) { + Info.set(MF.createMIExtraInfo( + MMOs, PreInstrSymbol, PostInstrSymbol, HeapAllocMarker)); return; } - if (!getPreInstrSymbol()) { - Info.set(getPostInstrSymbol()); + + // Otherwise store the single pointer inline. + if (HasPreInstrSymbol) + Info.set(PreInstrSymbol); + else if (HasPostInstrSymbol) + Info.set(PostInstrSymbol); + else + Info.set(MMOs[0]); +} + +void MachineInstr::dropMemRefs(MachineFunction &MF) { + if (memoperands_empty()) return; - } - // Otherwise allocate a fresh extra info with just these symbols. - Info.set( - MF.createMIExtraInfo({}, getPreInstrSymbol(), getPostInstrSymbol())); + setExtraInfo(MF, {}, getPreInstrSymbol(), getPostInstrSymbol(), + getHeapAllocMarker()); } void MachineInstr::setMemRefs(MachineFunction &MF, @@ -346,15 +367,8 @@ return; } - // Try to store a single MMO inline. - if (MMOs.size() == 1 && !getPreInstrSymbol() && !getPostInstrSymbol()) { - Info.set(MMOs[0]); - return; - } - - // Otherwise create an extra info struct with all of our info. - Info.set( - MF.createMIExtraInfo(MMOs, getPreInstrSymbol(), getPostInstrSymbol())); + setExtraInfo(MF, MMOs, getPreInstrSymbol(), getPostInstrSymbol(), + getHeapAllocMarker()); } void MachineInstr::addMemOperand(MachineFunction &MF, @@ -376,7 +390,8 @@ // instruction. We can do this whenever the pre- and post-instruction symbols // are the same (including null). if (getPreInstrSymbol() == MI.getPreInstrSymbol() && - getPostInstrSymbol() == MI.getPostInstrSymbol()) { + getPostInstrSymbol() == MI.getPostInstrSymbol() && + getHeapAllocMarker() == MI.getHeapAllocMarker()) { Info = MI.Info; return; } @@ -450,67 +465,42 @@ } void MachineInstr::setPreInstrSymbol(MachineFunction &MF, MCSymbol *Symbol) { - MCSymbol *OldSymbol = getPreInstrSymbol(); - if (OldSymbol == Symbol) + // Do nothing if old and new symbols are the same. + if (Symbol == getPreInstrSymbol()) 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()) { - Info.clear(); - return; - } - if (memoperands_empty()) { - assert(getPostInstrSymbol() && - "Should never have only a single symbol allocated out-of-line!"); - Info.set(getPostInstrSymbol()); - return; - } - - // Otherwise fallback on the generic update. - } else if (!Info || Info.is()) { - // If we don't have any other extra info, we can store this inline. - Info.set(Symbol); + // If there was only one symbol and we're removing it, just clear info. + if (!Symbol && Info.is()) { + Info.clear(); return; } - // Otherwise, allocate a full new set of extra info. - // FIXME: Maybe we should make the symbols in the extra info mutable? - Info.set( - MF.createMIExtraInfo(memoperands(), Symbol, getPostInstrSymbol())); + setExtraInfo(MF, memoperands(), Symbol, getPostInstrSymbol(), + getHeapAllocMarker()); } void MachineInstr::setPostInstrSymbol(MachineFunction &MF, MCSymbol *Symbol) { - MCSymbol *OldSymbol = getPostInstrSymbol(); - if (OldSymbol == Symbol) + // Do nothing if old and new symbols are the same. + if (Symbol == getPostInstrSymbol()) 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()) { - Info.clear(); - return; - } - if (memoperands_empty()) { - assert(getPreInstrSymbol() && - "Should never have only a single symbol allocated out-of-line!"); - Info.set(getPreInstrSymbol()); - return; - } - - // Otherwise fallback on the generic update. - } else if (!Info || Info.is()) { - // If we don't have any other extra info, we can store this inline. - Info.set(Symbol); + // If there was only one symbol and we're removing it, just clear info. + if (!Symbol && Info.is()) { + Info.clear(); return; } - // Otherwise, allocate a full new set of extra info. - // FIXME: Maybe we should make the symbols in the extra info mutable? - Info.set( - MF.createMIExtraInfo(memoperands(), getPreInstrSymbol(), Symbol)); + setExtraInfo(MF, memoperands(), getPreInstrSymbol(), Symbol, + getHeapAllocMarker()); +} + +void MachineInstr::setHeapAllocMarker(MachineFunction &MF, MDNode *Marker) { + // Do nothing if old and new symbols are the same. + if (Marker == getHeapAllocMarker()) + return; + + setExtraInfo(MF, memoperands(), getPreInstrSymbol(), getPostInstrSymbol(), + Marker); } void MachineInstr::cloneInstrSymbols(MachineFunction &MF, @@ -524,6 +514,7 @@ setPreInstrSymbol(MF, MI.getPreInstrSymbol()); setPostInstrSymbol(MF, MI.getPostInstrSymbol()); + setHeapAllocMarker(MF, MI.getHeapAllocMarker()); } uint16_t MachineInstr::mergeFlagsWith(const MachineInstr &Other) const { @@ -1710,6 +1701,13 @@ 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 --- a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp @@ -1238,10 +1238,9 @@ updateValueMap(CLI.CS->getInstruction(), CLI.ResultReg, CLI.NumResultRegs); // Set labels for heapallocsite call. - if (CLI.CS && CLI.CS->getInstruction()->hasMetadata("heapallocsite")) { - const MDNode *MD = CLI.CS->getInstruction()->getMetadata("heapallocsite"); - MF->addCodeViewHeapAllocSite(CLI.Call, MD); - } + if (CLI.CS) + if (MDNode *MD = CLI.CS->getInstruction()->getMetadata("heapallocsite")) + CLI.Call->setHeapAllocMarker(*MF, MD); return true; } diff --git a/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp b/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp --- a/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp @@ -910,10 +910,9 @@ 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()) - MF.addCodeViewHeapAllocSite(NewInsn, MD); - } + NewInsn->setHeapAllocMarker(MF, MD); GluedNodes.pop_back(); } @@ -923,9 +922,10 @@ if (HasDbg) ProcessSourceNode(SU->getNode(), DAG, Emitter, VRBaseMap, Orders, Seen, NewInsn); + if (MDNode *MD = DAG->getHeapAllocSite(SU->getNode())) { if (NewInsn && NewInsn->isCall()) - MF.addCodeViewHeapAllocSite(NewInsn, MD); + NewInsn->setHeapAllocMarker(MF, MD); } } diff --git a/llvm/test/CodeGen/X86/label-heapallocsite.ll b/llvm/test/CodeGen/X86/label-heapallocsite.ll --- 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=DAG,CHECK %s -; RUN: llc -O0 < %s | FileCheck --check-prefixes=FAST,CHECK %s +; RUN: llc < %s | FileCheck --check-prefixes=CHECK %s +; RUN: llc -O0 < %s | FileCheck --check-prefixes=CHECK %s ; Source to regenerate: ; $ clang -cc1 -triple x86_64-windows-msvc t.cpp -debug-info-kind=limited \ @@ -71,42 +71,33 @@ ; 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: .Lheapallocsite0: -; CHECK: callq *{{.*}}%rax{{.*}} -; CHECK: .Lheapallocsite1: +; CHECK: callq *{{.*}}%rax{{.*}} +; CHECK-NEXT: [[LABEL1:.Ltmp[0-9]+]]: ; CHECK-LABEL: call_multiple: # @call_multiple -; FastISel emits instructions in a different 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: callq alloc_foo +; CHECK-NEXT: [[LABEL3:.Ltmp[0-9]+]]: +; CHECK: callq alloc_foo +; CHECK-NEXT: [[LABEL5:.Ltmp[0-9]+]]: ; CHECK-LABEL: .short 4423 # Record kind: S_GPROC32_ID ; CHECK: .short 4446 # Record kind: S_HEAPALLOCSITE -; CHECK-NEXT: .secrel32 .Lheapallocsite0 -; CHECK-NEXT: .secidx .Lheapallocsite0 -; CHECK-NEXT: .short .Lheapallocsite1-.Lheapallocsite0 +; CHECK-NEXT: .secrel32 [[LABEL0:.Ltmp[0-9]+]] +; CHECK-NEXT: .secidx [[LABEL0]] +; CHECK-NEXT: .short [[LABEL1]]-[[LABEL0]] ; CHECK-NEXT: .long 3 ; CHECK: .short 4446 # Record kind: S_HEAPALLOCSITE -; CHECK-NEXT: .secrel32 .Lheapallocsite2 -; CHECK-NEXT: .secidx .Lheapallocsite2 -; CHECK-NEXT: .short .Lheapallocsite3-.Lheapallocsite2 +; CHECK-NEXT: .secrel32 [[LABEL2:.Ltmp[0-9]+]] +; CHECK-NEXT: .secidx [[LABEL2]] +; CHECK-NEXT: .short [[LABEL3]]-[[LABEL2]] ; CHECK-NEXT: .long 4096 ; CHECK: .short 4446 # Record kind: S_HEAPALLOCSITE -; CHECK-NEXT: .secrel32 .Lheapallocsite4 -; CHECK-NEXT: .secidx .Lheapallocsite4 -; CHECK-NEXT: .short .Lheapallocsite5-.Lheapallocsite4 +; CHECK-NEXT: .secrel32 [[LABEL4:.Ltmp[0-9]+]] +; CHECK-NEXT: .secidx [[LABEL4]] +; CHECK-NEXT: .short [[LABEL5]]-[[LABEL4]] ; 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 --- a/llvm/test/CodeGen/X86/taildup-heapallocsite.ll +++ b/llvm/test/CodeGen/X86/taildup-heapallocsite.ll @@ -8,8 +8,7 @@ ; f2(); ; } -; 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. +; In this case, block placement duplicates the heap allocation site. ; ModuleID = 't.cpp' source_filename = "t.cpp" @@ -37,15 +36,25 @@ ; CHECK-LABEL: taildupit: # @taildupit ; CHECK: testq ; CHECK: je -; CHECK: .Lheapallocsite0: ; CHECK: callq alloc -; CHECK: .Lheapallocsite1: +; CHECK-NEXT: [[L1:.Ltmp[0-9]+]] ; CHECK: jmp f2 # TAILCALL -; CHECK-NOT: Lheapallocsite ; CHECK: callq alloc -; CHECK-NOT: Lheapallocsite +; CHECK-NEXT: [[L3:.Ltmp[0-9]+]] ; 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 --- a/llvm/unittests/CodeGen/MachineInstrTest.cpp +++ b/llvm/unittests/CodeGen/MachineInstrTest.cpp @@ -9,6 +9,7 @@ #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" @@ -16,6 +17,8 @@ #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" @@ -125,6 +128,7 @@ : LLVMTargetMachine(Target(), "", Triple(""), "", "", TargetOptions(), Reloc::Static, CodeModel::Small, CodeGenOpt::Default), ST(*this) {} + ~BogusTargetMachine() override {} const TargetSubtargetInfo *getSubtargetImpl(const Function &) const override { @@ -135,6 +139,13 @@ BogusSubtarget ST; }; +static MCAsmInfo AsmInfo = MCAsmInfo(); + +std::unique_ptr createMCContext() { + return std::make_unique( + &AsmInfo, nullptr, nullptr, nullptr, nullptr, false); +} + std::unique_ptr createTargetMachine() { return std::make_unique(); } @@ -361,6 +372,135 @@ 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 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 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 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::value, "trivially copyable"); } // end namespace