TaoPan added a comment.
Thanks MaskRay for your review comments!
================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1752
+ else {
+ UniqueID = NextUniqueID++;
+ COMDATSymName = MBB.getParent()->getName();
----------------
MaskRay wrote:
> I think `UniqueID = NextUniqueID++;` is not needed.
I added comment (line 1752) to explain why add this line, this line referred to
ELF implementation, please refer to
https://github.com/llvm/llvm-project/blob/cb2a2ba8d64da5be3fac0ea90e406270e8a615bd/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp#L993
================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1756
+
+ // TODO: construct cold section in the case of section ID of MBB is
+ // MBBSectionID::ColdSectionID
----------------
MaskRay wrote:
> Can you fix the TODO in this patch?
It's indeed TODO. MMBSectionID::ColdSectionID will be handled in MFS on Windows
COFF patch. I plan to re-base my previous MFS on Windows COFF patch (D95209)
after this patch. I also explained this TODO in Summary.
================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1786
+ // COFF linker will not properly handle comdats otherwise.
+ if (getContext().getTargetTriple().isWindowsGNUEnvironment()) {
+ Name += '$';
----------------
MaskRay wrote:
> Is `isOSBinFormatCOFF()` more appropriate? Can windows-gnu use the
> optimization?
Windows-gnu can use the optimization. In the case of Windows-msvc,
isOSBinFormatCOFF() is true, but the section name should not include
COMDATSymName suffix.
This part referred to existing code
https://github.com/llvm/llvm-project/blob/cb2a2ba8d64da5be3fac0ea90e406270e8a615bd/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp#L1663
Thanks for this Windows-gnu comment! I reviewed Window-gnu related
implementation and fixed Windows-gnu problem of
TargetLoweringObjectFileCOFF::getSectionForMachineBasicBlock().
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99487/new/
https://reviews.llvm.org/D99487
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
