[PATCH] D92073: [CodeGen] Add text section prefix for COFF object file

Thu Dec 3 23:39:33 PST 2020

TaoPan added inline comments.

================
Comment at: llvm/test/CodeGen/X86/text-section-prefix.ll:20
+; COFF-MSVC:  .section  .text$unlikely,"xr",one_only,foo2
+; COFF-GNU:   .section  .text$unlikely$foo2,"xr",one_only,foo2
+  ret void
----------------
mstorsjo wrote:
> TaoPan wrote:
> > mstorsjo wrote:
> > > rnk wrote:
> > > > @mstorsjo This seems a bit more complicated than I originally thought it would be.
> > > Yeah, but I guess this is what we end up with, if we append things this way. I wonder whether this works properly when interacting with GNU ld?
> > > 
> > > The patch description has very little context about the what/where/why - so this is something that ends up invoked only if building with PGO?
> > Thanks for your review comments!
> > COFF support "$" character as a special interpretation in section names for grouped sections. If GNU ld support COFF, it should recognize "$" and works properly.
> > I added some description to the end of Summary. Yes, it will be invoked only if building with PGO, but the benefit is not limited to PGO, it's also make possible aggregate other compilers e.g. v8 created same prefix sections. 
> GNU ld does support COFF, but it has its own interpretation of some bits regarding to COMDAT sections, deviating a bit from the spec.
> 
> I'll try to test this usecase, before and after this patch, with both lld and GNU ld, to see if there are issues - I hope you're ok with waiting until I've had a change to try it.
Sure, I'm ok with waiting for your test, and thanks for your test!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92073