- User Since
- Jun 10 2017, 12:15 PM (171 w, 3 d)
Ideally we should only have to specify this once. However, using function attributes doesn't seem ideal since the pass will be scheduled and repeatedly invoked only to return without actually running the pass. It would be cleaner to marshal the codegen specific options from the compile invocation and restore them for the LTO step. There are a couple of other codegen options which would also benefit from this approach --lto-unique-basic-block-section-names, --lto-basic-block-sections=<value>. @mtrofin pointed out that -fembed-bitcode saves the invocation in .llvmcmd. A similar approach to stash codegen specific options always for LTO to pick up and enable might be less intrusive. However, this is a larger effort and for current LTO builds it would be nice to have a command line option to enable it. WDYT about this alternative?
Mon, Sep 21
Rebase and update git commit message.
Fri, Sep 18
Thanks for the quick review!
Drop braces, add a test.
Thu, Sep 17
Carrying over the discussion from D87813 since it's more appropriate here:
Drop lld/ELF/Writer.cpp changes.
Wed, Sep 16
Tue, Sep 15
Thanks for the review.
Update the test.
Mon, Sep 14
Remove unnecessary includes, update doc text.
That makes sense. I moved the check to lib/Driver/ToolChains/Clang.cpp and updated the test. Seems cleaner to have all the checks in one place.
Check profile flag in Driver, update test.
Remove extra -c from test command line.
@MaskRay ping, let me know if you have any further comments. Thanks!
Fri, Sep 11
It feels wrong that the assembly+llvm-profdata test is in clang/test
I agree with @dblaikie and your assessment that it feels wrong to add such a test to clang. In the first version of this patch, the test served the purpose of representing the canonical usage with profdata as well as check for the plumbing of the profile along with the presence of the flag. Enhancing the driver test removed the need for the former while adding a diagnostic warning message (and a check for it) removes the need for the latter. I've removed the clang codegen test.
Remove clang/CodeGen test, update arg render logic.
Thu, Sep 10
Fix test formatting.
Use OptInFFlag, split-file and update tests.
Check warning, specify target to avoid failures on windows.
Updated test and warning type.
Thanks for explaining the rationale, PTAL.
Update test to use not tool and -c flag.
Wed, Sep 9
Thanks for the quick review @MaskRay, PTAL.
Update test based on review comments.
Specify triple for driver tests, address comments.
Add a check for x86-elf.
Tue, Sep 8
Once this patch is in we can look into splitting ehpads out though I'm more inclined to enhance the static profile count mechanism to account for ehpads appropriately rather than adding a new flag to MFS. In general, it would be great to have MFS operate with only the knowledge of profiles counts rather than special-casing for particular types of blocks. I tried to test static count driven ehpad splitting on mysql but applying the patch to ToT fails (AsmPrinter.cpp needs to be rebased).
Thu, Sep 3
Thanks for the comments! PTAL.
Add warning when option is enabled without profile.
Wed, Sep 2
Fri, Aug 28
LGTM. Please wait a bit for additional comments from others.
Thanks for the comments all! The builds look green and I'm going to go ahead and push this.
Address reviewer comments.
Wed, Aug 26
Tue, Aug 25
Add a comment to explain renumbering, FIXME for ehpads.
Aug 20 2020
Aug 18 2020
Thanks for the reviews!
@hiraditya I've added a test to ensure ehpads are not split out. Let me know if there are additional cases you want to cover.
Address review comments.
Aug 10 2020
Add an option to exclude specific sections.
I've uploaded a Makefile here which will allow you to run the bootstrap benchmarks. Applying this patch on a local llvm repo and pointing the Makefile at it should be sufficient to get you going.
Aug 8 2020
Update PSI metadata to fix assert failure.
Aug 7 2020
Could you share the details of the machine as well?
Sure, these were measured on a Lenovo P920 workstation -- Intel Skylake based Xeon(R) Gold 6154 CPU.
Simplify the cold count check.
Updated diff based on review comments.
Aug 6 2020
Aug 5 2020
Aug 4 2020
May 4 2020
May 2 2020
Updated string types based on reviewer comments.
@efriedma Thanks for the pointers on LLVM string types. Updated the diff with changes, please take a look. Thanks!
May 1 2020
Apr 30 2020
Apr 24 2020
Apr 23 2020
Addressed review comment.
Use a ".text.eh" prefix instead of ".eh" suffix for exception machine basic block sections.
Update the basic-block-sections-clusters-eh test.