[llvm] bc7595d - [StackMaps] Be explicit about label formation [NFC]
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 19 13:19:42 PST 2019
Thanks!
(This was a definite "doh!" on my part. Will make the same API change
for all users, build all targets, and recommit.)
Philip
On 12/19/19 12:53 PM, Eric Christopher wrote:
> Went ahead and reverted thusly:
>
> echristo at athyra ~/r/llvm-project> git push
> To github.com:llvm/llvm-project.git
> 3346cecd4c0..add710eb23d master -> master
>
> On Thu, Dec 19, 2019 at 12:49 PM Eric Christopher <echristo at gmail.com
> <mailto:echristo at gmail.com>> wrote:
>
> Hi Philip,
>
> This broke the aarch64 build.
>
> -eric
>
> On Thu, Dec 19, 2019 at 12:39 PM Philip Reames via llvm-commits
> <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>>
> wrote:
>
>
> Author: Philip Reames
> Date: 2019-12-19T12:38:44-08:00
> New Revision: bc7595d934b958ab481288d7b8e768fe5310be8f
>
> URL:
> https://github.com/llvm/llvm-project/commit/bc7595d934b958ab481288d7b8e768fe5310be8f
> DIFF:
> https://github.com/llvm/llvm-project/commit/bc7595d934b958ab481288d7b8e768fe5310be8f.diff
>
> LOG: [StackMaps] Be explicit about label formation [NFC]
>
> For auto-padding assembler support, we'll need to bundle the
> label with the instructions (nops or call sequences) so that
> they don't get separated. This just rearranges the code to
> make the upcoming change more obvious.
>
> Added:
>
>
> Modified:
> llvm/include/llvm/CodeGen/StackMaps.h
> llvm/lib/CodeGen/StackMaps.cpp
> llvm/lib/Target/X86/X86MCInstLower.cpp
>
> Removed:
>
>
>
> ################################################################################
> diff --git a/llvm/include/llvm/CodeGen/StackMaps.h
> b/llvm/include/llvm/CodeGen/StackMaps.h
> index d7d88de6f682..63547e5b7c3e 100644
> --- a/llvm/include/llvm/CodeGen/StackMaps.h
> +++ b/llvm/include/llvm/CodeGen/StackMaps.h
> @@ -266,13 +266,16 @@ class StackMaps {
> /// Generate a stackmap record for a stackmap instruction.
> ///
> /// MI must be a raw STACKMAP, not a PATCHPOINT.
> - void recordStackMap(const MachineInstr &MI);
> + void recordStackMap(const MCSymbol &L,
> + const MachineInstr &MI);
>
> /// Generate a stackmap record for a patchpoint instruction.
> - void recordPatchPoint(const MachineInstr &MI);
> + void recordPatchPoint(const MCSymbol &L,
> + const MachineInstr &MI);
>
> /// Generate a stackmap record for a statepoint instruction.
> - void recordStatepoint(const MachineInstr &MI);
> + void recordStatepoint(const MCSymbol &L,
> + const MachineInstr &MI);
>
> /// If there is any stack map data, create a stack map
> section and serialize
> /// the map info into it. This clears the stack map data
> structures
> @@ -306,12 +309,15 @@ class StackMaps {
> /// registers that need to be recorded in the stackmap.
> LiveOutVec parseRegisterLiveOutMask(const uint32_t *Mask)
> const;
>
> - /// This should be called by the MC lowering code
> _immediately_ before
> - /// lowering the MI to an MCInst. It records where the
> operands for the
> - /// instruction are stored, and outputs a label to record
> the offset of
> - /// the call from the start of the text section. In special
> cases (e.g. AnyReg
> - /// calling convention) the return register is also
> recorded if requested.
> - void recordStackMapOpers(const MachineInstr &MI, uint64_t ID,
> + /// Record the locations of the operands of the provided
> instruction in a
> + /// record keyed by the provided label. For instructions
> w/AnyReg calling
> + /// convention the return register is also recorded if
> requested. For
> + /// STACKMAP, and PATCHPOINT the label is expected to
> immediately *preceed*
> + /// lowering of the MI to MCInsts. For STATEPOINT, it
> expected to
> + /// immediately *follow*. It's not clear this
> diff erence was intentional,
> + /// but it exists today.
> + void recordStackMapOpers(const MCSymbol &L,
> + const MachineInstr &MI, uint64_t ID,
> MachineInstr::const_mop_iterator MOI,
> MachineInstr::const_mop_iterator MOE,
> bool recordResult = false);
>
> diff --git a/llvm/lib/CodeGen/StackMaps.cpp
> b/llvm/lib/CodeGen/StackMaps.cpp
> index 23a3cb7cc7f9..e16587c44a55 100644
> --- a/llvm/lib/CodeGen/StackMaps.cpp
> +++ b/llvm/lib/CodeGen/StackMaps.cpp
> @@ -294,14 +294,13 @@
> StackMaps::parseRegisterLiveOutMask(const uint32_t *Mask) const {
> return LiveOuts;
> }
>
> -void StackMaps::recordStackMapOpers(const MachineInstr &MI,
> uint64_t ID,
> +void StackMaps::recordStackMapOpers(const MCSymbol &MILabel,
> + const MachineInstr &MI,
> uint64_t ID,
> MachineInstr::const_mop_iterator MOI,
> MachineInstr::const_mop_iterator MOE,
> bool recordResult) {
> MCContext &OutContext = AP.OutStreamer->getContext();
> - MCSymbol *MILabel = OutContext.createTempSymbol();
> - AP.OutStreamer->EmitLabel(MILabel);
> -
> +
> LocationVec Locations;
> LiveOutVec LiveOuts;
>
> @@ -340,7 +339,7 @@ void StackMaps::recordStackMapOpers(const
> MachineInstr &MI, uint64_t ID,
> // Create an expression to calculate the offset of the
> callsite from function
> // entry.
> const MCExpr *CSOffsetExpr = MCBinaryExpr::createSub(
> - MCSymbolRefExpr::create(MILabel, OutContext),
> + MCSymbolRefExpr::create(&MILabel, OutContext),
> MCSymbolRefExpr::create(AP.CurrentFnSymForSize,
> OutContext), OutContext);
>
> CSInfos.emplace_back(CSOffsetExpr, ID, std::move(Locations),
> @@ -360,22 +359,23 @@ void
> StackMaps::recordStackMapOpers(const MachineInstr &MI,
> uint64_t ID,
> FnInfos.insert(std::make_pair(AP.CurrentFnSym,
> FunctionInfo(FrameSize)));
> }
>
> -void StackMaps::recordStackMap(const MachineInstr &MI) {
> +void StackMaps::recordStackMap(const MCSymbol &L, const
> MachineInstr &MI) {
> assert(MI.getOpcode() == TargetOpcode::STACKMAP &&
> "expected stackmap");
>
> StackMapOpers opers(&MI);
> const int64_t ID =
> MI.getOperand(PatchPointOpers::IDPos).getImm();
> - recordStackMapOpers(MI, ID, std::next(MI.operands_begin(),
> opers.getVarIdx()),
> + recordStackMapOpers(L, MI, ID, std::next(MI.operands_begin(),
> + opers.getVarIdx()),
> MI.operands_end());
> }
>
> -void StackMaps::recordPatchPoint(const MachineInstr &MI) {
> +void StackMaps::recordPatchPoint(const MCSymbol &L, const
> MachineInstr &MI) {
> assert(MI.getOpcode() == TargetOpcode::PATCHPOINT &&
> "expected patchpoint");
>
> PatchPointOpers opers(&MI);
> const int64_t ID = opers.getID();
> auto MOI = std::next(MI.operands_begin(),
> opers.getStackMapStartIdx());
> - recordStackMapOpers(MI, ID, MOI, MI.operands_end(),
> + recordStackMapOpers(L, MI, ID, MOI, MI.operands_end(),
> opers.isAnyReg() && opers.hasDef());
>
> #ifndef NDEBUG
> @@ -390,14 +390,14 @@ void StackMaps::recordPatchPoint(const
> MachineInstr &MI) {
> #endif
> }
>
> -void StackMaps::recordStatepoint(const MachineInstr &MI) {
> +void StackMaps::recordStatepoint(const MCSymbol &L, const
> MachineInstr &MI) {
> assert(MI.getOpcode() == TargetOpcode::STATEPOINT &&
> "expected statepoint");
>
> StatepointOpers opers(&MI);
> // Record all the deopt and gc operands (they're contiguous
> and run from the
> // initial index to the end of the operand list)
> const unsigned StartIdx = opers.getVarIdx();
> - recordStackMapOpers(MI, opers.getID(), MI.operands_begin()
> + StartIdx,
> + recordStackMapOpers(L, MI, opers.getID(),
> MI.operands_begin() + StartIdx,
> MI.operands_end(), false);
> }
>
>
> diff --git a/llvm/lib/Target/X86/X86MCInstLower.cpp
> b/llvm/lib/Target/X86/X86MCInstLower.cpp
> index 4876df50766d..91001a3c4c6a 100644
> --- a/llvm/lib/Target/X86/X86MCInstLower.cpp
> +++ b/llvm/lib/Target/X86/X86MCInstLower.cpp
> @@ -1194,7 +1194,10 @@ void
> X86AsmPrinter::LowerSTATEPOINT(const MachineInstr &MI,
>
> // Record our statepoint node in the same section used by
> STACKMAP
> // and PATCHPOINT
> - SM.recordStatepoint(MI);
> + auto &Ctx = OutStreamer->getContext();
> + MCSymbol *MILabel = Ctx.createTempSymbol();
> + OutStreamer->EmitLabel(MILabel);
> + SM.recordStatepoint(*MILabel, MI);
> }
>
> void X86AsmPrinter::LowerFAULTING_OP(const MachineInstr
> &FaultingMI,
> @@ -1286,7 +1289,12 @@ void
> X86AsmPrinter::LowerPATCHABLE_OP(const MachineInstr &MI,
> // <id>, <shadowBytes>, ...
> void X86AsmPrinter::LowerSTACKMAP(const MachineInstr &MI) {
> SMShadowTracker.emitShadowPadding(*OutStreamer,
> getSubtargetInfo());
> - SM.recordStackMap(MI);
> +
> + auto &Ctx = OutStreamer->getContext();
> + MCSymbol *MILabel = Ctx.createTempSymbol();
> + OutStreamer->EmitLabel(MILabel);
> +
> + SM.recordStackMap(*MILabel, MI);
> unsigned NumShadowBytes = MI.getOperand(1).getImm();
> SMShadowTracker.reset(NumShadowBytes);
> }
> @@ -1299,7 +1307,10 @@ void
> X86AsmPrinter::LowerPATCHPOINT(const MachineInstr &MI,
>
> SMShadowTracker.emitShadowPadding(*OutStreamer,
> getSubtargetInfo());
>
> - SM.recordPatchPoint(MI);
> + auto &Ctx = OutStreamer->getContext();
> + MCSymbol *MILabel = Ctx.createTempSymbol();
> + OutStreamer->EmitLabel(MILabel);
> + SM.recordPatchPoint(*MILabel, MI);
>
> PatchPointOpers opers(&MI);
> unsigned ScratchIdx = opers.getNextScratchIdx();
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191219/9fdd42c4/attachment.html>
More information about the llvm-commits
mailing list