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

TaoPan via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 1 17:41:38 PST 2020


TaoPan added inline comments.


================
Comment at: llvm/test/CodeGen/X86/text-section-prefix.ll:9
+;; CHECK hot section name
+; SECTION-HOT-ELF:  .section        .text.hot
+; SECTION-HOT-COFF: .section        .text$hot
----------------
rnk wrote:
> TaoPan wrote:
> > rnk wrote:
> > > rnk wrote:
> > > > TaoPan wrote:
> > > > > pengfei wrote:
> > > > > > TaoPan wrote:
> > > > > > > pengfei wrote:
> > > > > > > > You don't need to distinguish `HOT` and `UNLIKLY` since they are in different functions. You just need `SECTION-ELF` and `SECTION-COFF`.
> > > > > > > > You may also need to add
> > > > > > > > ```
> > > > > > > > ; RUN: llc -mtriple x86_64-linux-gnu -unique-section-names=0 %s -o - | FileCheck %s --check-prefix=ELF-NOUNIQ
> > > > > > > > ; RUN: llc -mtriple x86_64-windows-mingw64 %s -o - | FileCheck %s --check-prefix=MINGW
> > > > > > > > ```
> > > > > > > > as Reid suggested.
> > > > > > > Thanks for your review comments! I removed HOT and UNLIKELY, and added ELF-NOUNIQ and MINGW, could you please have a look?
> > > > > > Nit: It seems no difference between `MSVC` and `MINGW`, you can use `COFF` for both to reduce the duplicated check.
> > > > > Thanks! It's good idea, I combined MSVC and MINGW into COFF.
> > > > They will differ if you use a linkonce_odr comdat function. These are very common in C++, it is worth testing this case. Try:
> > > > 
> > > > ```
> > > > $foocomdat = comdat any
> > > > define linkonce_odr dso_local i32 @foocomdat(i32 %0, i32 %1) comdat {
> > > >   %3 = add nsw i32 %1, %0
> > > >   ret i32 %3
> > > > }
> > > > ```
> > > > The sections for foocomdat will have a suffix on mingw.
> > > Perhaps use weak_odr linkage instead, so that it is non-discardable and is not dropped by DCE.
> > Thanks for your review comment!
> > I tried you suggested case, and replaced linkonce_odr with weak_odr, I didn't find difference between x86_64-windows-msvc and x86_64-windows-mingw64.
> > Is the difference related to text section name? I think this patch doesn't change anything except text section name.
> Yes, there should be a difference. Consider:
> 
> ```
> $ cat t.cpp 
> inline int myadd(int x, int y) { return x + y; }
> int main() { return myadd(1, 2); }
> 
> $ clang -S -O0 --target=x86_64-windows-msvc t.cpp  -o - | grep .text
>         .text
>         .section        .text,"xr",discard,"?myadd@@YAHHH at Z"
> 
> $ clang -S -O0 --target=x86_64-windows-gnu t.cpp  -o - | grep .text
>         .text
>         .section        .text$_Z5myaddii,"xr",discard,_Z5myaddii
> ```
> 
> Note how in the gnu environment LLVM appends the symbol name to the section name. Did you add the comdat as well?
Thanks for your notice of windows gnu environment and example!
I can make the difference after changing target to x86_64-windows-**gnu** (referred to your example).
Could you please have a look again?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92073



More information about the llvm-commits mailing list