[PATCH] D65023: [MS] Pass S_HEAPALLOCSITE metadata through SelectionDAG

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:4061
     DAG.addCallSiteInfo(Ret.getNode(), std::move(CSInfo));
+    if (CLI.CS && CLI.CS->getMetadata("heapallocsite"))
+      DAG.addHeapAllocSite(Ret.getNode(), CLI.CS->getMetadata("heapallocsite"));
----------------
Code golf suggestion, saves a hash lookup:
  if (CLI.CS)
    if (MDNode *HeapAlloc = CLI.CS->getMetatata("heapallocsite"))
      DAG.addHeapAllocSite(Ret.getNode(), HeapAlloc);


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:4062
+    if (CLI.CS && CLI.CS->getMetadata("heapallocsite"))
+      DAG.addHeapAllocSite(Ret.getNode(), CLI.CS->getMetadata("heapallocsite"));
+
----------------
I thought testing showed that MSVC didn't emit these records for tail calls. If we simply do nothing here, we'd skip the S_HEAPALLOCSITE record, and things would keep working, right? Any reason not to do that?


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:4077
+  if (CLI.CS && CLI.CS->getMetadata("heapallocsite"))
+    DAG.addHeapAllocSite(Chain.getNode(), CLI.CS->getMetadata("heapallocsite"));
+
----------------
Same suggestion


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65023/new/

https://reviews.llvm.org/D65023