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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1752
+  else {
+    UniqueID = NextUniqueID++;
+    COMDATSymName = MBB.getParent()->getName();
----------------
I think `UniqueID = NextUniqueID++;` is not needed.


================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1756
+
+  // TODO: construct cold section in the case of section ID of MBB is
+  // MBBSectionID::ColdSectionID
----------------
Can you fix the TODO in this patch?


================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1786
+  // COFF linker will not properly handle comdats otherwise.
+  if (getContext().getTargetTriple().isWindowsGNUEnvironment()) {
+    Name += '$';
----------------
Is `isOSBinFormatCOFF()` more appropriate? Can windows-gnu use the optimization?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99487