[PATCH] D99487: [CodeGen] Port basic block sections from ELF to COFF

[Sriraman Tallam via Phabricator via llvm-commits]

tmsriram added inline comments.


================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1711
+      COFF::IMAGE_SCN_CNT_CODE | COFF::IMAGE_SCN_MEM_EXECUTE |
+          COFF::IMAGE_SCN_MEM_READ | COFF::IMAGE_SCN_LNK_COMDAT,
+      SectionKind::getText(), COMDATSymName,
----------------
Why is this set to always comdat? This should be comdat only if the function is comdat, right?


================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1712
+          COFF::IMAGE_SCN_MEM_READ | COFF::IMAGE_SCN_LNK_COMDAT,
+      SectionKind::getText(), COMDATSymName,
+      COFF::IMAGE_COMDAT_SELECT_NODUPLICATES, UniqueID);
----------------
COMDATSymName can be folded to be equal to MBB.getSymbol()->getName() here?  Plus, you are not preserving the .text.hot prefix that the original function might get here.  Is this future work?  If the original function is named .text.hot.foo, the basic block will still be named .text.foo.__part.1 which is not right.

Plus, what about exception handling sections like ".eh.*"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99487