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