Text section prefix is created in CodeGenPrepare, it's file format independent implementation, text section name is written into object file in TargetLoweringObjectFile, it's file format dependent implementation, port code of adding text section prefix to text section name from ELF to COFF.
Different with ELF that use '.' as concatenation character, COFF use '$' as concatenation character. That is, concatenation character is variable, so split concatenation character from text section prefix.
Text section prefix is existing feature of ELF, it can help to reduce icache and itlb misses, it's also make possible aggregate other compilers e.g. v8 created same prefix sections. Furthermore, the recent feature Machine Function Splitter (basic block level text prefix section) is based on text section prefix.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
Can you add more detail for the background on summary?
llvm/test/CodeGen/X86/text-section-prefix.ll | ||
---|---|---|
2 | What's your checking here? |
llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp | ||
---|---|---|
623 | This is out of scope for the patch, but why is it called a "section prefix" if it goes after the section name? ".text.hot._ZfooEv" | |
llvm/test/CodeGen/X86/text-section-prefix.ll | ||
2 | Please run this test in a few configurations. You can add two RUN lines like: ; RUN: llc -mtriple x86_64-linux-gnu %s -o - | FileCheck %s --check-prefix=ELF ; 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-msvc %s -o - | FileCheck %s --check-prefix=MSVC ; RUN: llc -mtriple x86_64-windows-msvc %s -o - | FileCheck %s --check-prefix=MINGW Add check lines for the section directives. |
llvm/test/CodeGen/X86/text-section-prefix.ll | ||
---|---|---|
4 | You may want some RUN configurations in Transforms/CodeGenPrepare/X86/section.ll |
llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp | ||
---|---|---|
623 | Without this modification, ".texthot._ZfooEv", no '.' between .text and hot, as the '.' was removed in CodeGenPrepare.cpp | |
llvm/test/CodeGen/X86/text-section-prefix.ll | ||
2 | I added some description in Summary. The line of output .section .text,xxx will be changed to .section .text**$hot**,xxx or .section .text**$unlikely**,xxx with this patch on Windows. | |
2 | Thanks for your nice guidance! | |
4 | Thanks! I'll refer to this file. |
llvm/test/CodeGen/X86/text-section-prefix.ll | ||
---|---|---|
2 | I see. Then you just need to add RUNs as Reid suggested and checks like what in section.ll |
llvm/test/CodeGen/X86/text-section-prefix.ll | ||
---|---|---|
2 | ok, I'll add RUNs and checks as Reid and MaskRay suggested, thanks! |
llvm/test/CodeGen/X86/text-section-prefix.ll | ||
---|---|---|
1 | Maybe better to put the test in Transforms/CodeGenPrepare/X86 | |
9 | You don't need to distinguish HOT and UNLIKLY since they are in different functions. You just need SECTION-ELF and SECTION-COFF. ; 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. |
llvm/test/CodeGen/X86/text-section-prefix.ll | ||
---|---|---|
9 | Thanks for your review comments! I removed HOT and UNLIKELY, and added ELF-NOUNIQ and MINGW, could you please have a look? |
LGTM. You'd better wait few days to see if others object. Thanks.
llvm/test/CodeGen/X86/text-section-prefix.ll | ||
---|---|---|
9 | Nit: It seems no difference between MSVC and MINGW, you can use COFF for both to reduce the duplicated check. |
llvm/test/CodeGen/X86/text-section-prefix.ll | ||
---|---|---|
9 | Thanks! It's good idea, I combined MSVC and MINGW into COFF. |
llvm/test/CodeGen/X86/text-section-prefix.ll | ||
---|---|---|
9 | 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. |
llvm/test/CodeGen/X86/text-section-prefix.ll | ||
---|---|---|
9 | Perhaps use weak_odr linkage instead, so that it is non-discardable and is not dropped by DCE. |
llvm/test/CodeGen/X86/text-section-prefix.ll | ||
---|---|---|
9 | Thanks for your review comment! |
llvm/test/CodeGen/X86/text-section-prefix.ll | ||
---|---|---|
9 | 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@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? |
Change x86_64-windows-mingw64 to x86_64-windows-gnu for mingw format text section name
llvm/test/CodeGen/X86/text-section-prefix.ll | ||
---|---|---|
9 | Thanks for your notice of windows gnu environment and example! |
llvm/test/CodeGen/X86/text-section-prefix.ll | ||
---|---|---|
1 | Sorry! I didn't note and reply this comment previously. |
I think this is an improvement, so let's go ahead and land it, but I'm not sure this will really work for mingw.
One last think I will note is that COFF linkers will sort input sections by the part of the section name after the dollar sign. There isn't any special treatment of .text$hot / .text$unlikely. They are sorted early and late by their ASCII ordering. That seems to achieve the right behavior, but maybe PGO should use more intentional section prefixes to get the desired ordering.
llvm/test/CodeGen/X86/text-section-prefix.ll | ||
---|---|---|
21 | @mstorsjo This seems a bit more complicated than I originally thought it would be. | |
llvm/test/Transforms/CodeGenPrepare/X86/section.ll | ||
47 | The section name is .text.unlikely., with a trailing dot. I guess that is the existing behavior, but it seems odd to me. Nevermind. |
llvm/test/CodeGen/X86/text-section-prefix.ll | ||
---|---|---|
21 | 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? |
llvm/test/CodeGen/X86/text-section-prefix.ll | ||
---|---|---|
21 | Thanks for your review comments! |
llvm/test/Transforms/CodeGenPrepare/X86/section.ll | ||
---|---|---|
47 | Yes, the existing code } else if (HasPrefix) Name.push_back('.'); leads to the trailing dot. |
llvm/test/CodeGen/X86/text-section-prefix.ll | ||
---|---|---|
21 | 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. |
llvm/test/CodeGen/X86/text-section-prefix.ll | ||
---|---|---|
21 | Sure, I'm ok with waiting for your test, and thanks for your test! |
llvm/test/CodeGen/X86/text-section-prefix.ll | ||
---|---|---|
21 | Ok, so I've tested this in a few mingw configurations, and it does seem to work, with a few caveats: If a comdat symbol is marked "unlikely" in one translation unit but not in another one, linking fails with GNU ld (but lld still succeeds). So for GNU ld, it seems like the section suffix ($unlikely$comdatname) needs to be identical in all occurrances of the symbol/section for the comdat deduplication to work. In lld it works as intended via the leader symbol. For comdats with associated sections, like .text$unlikely$comdatname associated with .xdata$unlikey$comdatname, associativity seems to work fine as long as the suffix, including $unlikely is identical between .text, .pdata and .xdata - which it turns out to be. But maybe it'd be good to have a test explicitly check that it really is the case? |
llvm/test/CodeGen/X86/text-section-prefix.ll | ||
---|---|---|
21 | I added test case to llvm/test/CodeGen/X86/mingw-comdats.ll for checking the order of .text$hot$comdatname and .text$unlikely$comdatname with existing sections, could you please have a look? |
llvm/test/CodeGen/X86/mingw-comdats.ll | ||
---|---|---|
88 ↗ | (On Diff #309803) | I'm not quite sure what this test actually is supposed to test; ordering of sections according to the suffixes doesn't happen at the object file stage, that only happens while linking. What would be useful, though, would be to add a new function similar to the _Z3fooi one above, with e.g. a unlikely attribute, and check that all of .text/.xdata/.pdata get similar suffixes just like the checks right above on lines 80-83. |
Update mingw-comdats.ll to check .text$unlikely$comdatname,
.xdata$unlikely$comdatname and .pdata$unlikely$comdatname
llvm/test/CodeGen/X86/mingw-comdats.ll | ||
---|---|---|
88 ↗ | (On Diff #309803) | Thanks for your detail guidance! |
This is out of scope for the patch, but why is it called a "section prefix" if it goes after the section name? ".text.hot._ZfooEv"