[PATCH] D92073: [CodeGen] Add text section prefix for COFF object file
Reid Kleckner via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 1 08:20:08 PST 2020
rnk 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
----------------
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?
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