[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

This revision was automatically updated to reflect the committed changes. Closed by commit rGf1a3ab904439: [clang] Add a command line flag for the Machine Function Splitter. (authored by snehasish). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

snehasish added a comment. Thanks for the review. Comment at: clang/include/clang/Driver/Options.td:2000 +defm split_machine_functions: OptInFFlag<"split-machine-functions", + "Enable", "Disable", " late function splitting using profile information (x86-elf only)">; +

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

snehasish updated this revision to Diff 291986. snehasish marked 2 inline comments as done. snehasish added a comment. Update the test. - Added "warning: " prefix to be checked. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87047/new/

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Driver/Options.td:2000 +defm split_machine_functions: OptInFFlag<"split-machine-functions", + "Enable", "Disable", " late function

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

snehasish updated this revision to Diff 291775. snehasish added a comment. Remove unnecessary includes, update doc text. - Update doctext. - Remove no longer necessary includes to frontend driver diagnostics. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:2000 +defm split_machine_functions: OptInFFlag<"split-machine-functions", + "Enable", "Disable", " late function splitting using profile information (x86-elf only)">; + If this

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

snehasish marked 3 inline comments as done. snehasish added a comment. 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. PTAL, thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

snehasish updated this revision to Diff 291771. snehasish marked an inline comment as done. snehasish added a comment. Check profile flag in Driver, update test. - Move the profile missing warning check to the Driver. - Update the test to use -### for CHECK-WARN. Repository: rG LLVM Github

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

MaskRay added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:518 + if (CodeGenOpts.SplitMachineFunctions) { +if (CodeGenOpts.getProfileUse() != CodeGenOptions::ProfileNone) I did not pay attention. Such compatibility checks should be added

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

MaskRay requested changes to this revision. MaskRay added inline comments. This revision now requires changes to proceed. Comment at: clang/test/Driver/fsplit-machine-functions.c:3 +// RUN: %clang -### -target x86_64 -fprofile-use=default.profdata -fsplit-machine-functions

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

snehasish marked an inline comment as done. snehasish added inline comments. Comment at: clang/test/Driver/fsplit-machine-functions.c:3 +// RUN: %clang -### -target x86_64 -fprofile-use=default.profdata -fsplit-machine-functions -fno-split-machine-functions %s -c 2>&1 |

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

snehasish updated this revision to Diff 291758. snehasish added a comment. Remove extra -c from test command line. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87047/new/ https://reviews.llvm.org/D87047 Files:

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/Driver/fsplit-machine-functions.c:3 +// RUN: %clang -### -target x86_64 -fprofile-use=default.profdata -fsplit-machine-functions

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

snehasish added a comment. @MaskRay ping, let me know if you have any further comments. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87047/new/ https://reviews.llvm.org/D87047 ___ cfe-commits

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

snehasish updated this revision to Diff 291728. snehasish added a comment. Rebased patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87047/new/ https://reviews.llvm.org/D87047 Files: clang/include/clang/Basic/CodeGenOptions.def

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

snehasish marked 3 inline comments as done. snehasish added a subscriber: dblaikie. snehasish added a comment. > 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

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

snehasish updated this revision to Diff 291292. snehasish added a comment. Remove clang/CodeGen test, update arg render logic. - Removed the clang/CodeGen test. - Added a check for the option to be rendered. - Fixed extra flags in driver test. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

MaskRay added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:2131 +.. option:: -fsplit-machine-functions, -fno-split-machine-functions + You don't need to edit this file. It is auto-generated from Options.td Comment

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

snehasish marked an inline comment as done. snehasish added inline comments. Comment at: clang/test/CodeGen/split-machine-functions.c:3 + +// RUN: echo "foo"> %t.proftext +// RUN: echo "# Func Hash:" >> %t.proftext snehasish wrote: >

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

snehasish updated this revision to Diff 291122. snehasish marked an inline comment as done. snehasish added a comment. Fix test formatting. Added clang-format off to disable clang format for the test which contains both profile data (as text) and c code. Repository: rG LLVM Github Monorepo

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

snehasish marked 3 inline comments as done. snehasish added a comment. PTAL, thanks! Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4259 options::OPT_fno_unique_basic_block_section_names, +options::OPT_fsplit_machine_functions, +

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

snehasish updated this revision to Diff 291108. snehasish added a comment. Use OptInFFlag, split-file and update tests. - Change the flag type to OptInFFlag. - Use split-file in the test to avoid "RUN: echo" lines. - Use an existing warn message (if no profile is available) and add a check for

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

MaskRay requested changes to this revision. MaskRay added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/Driver/Options.td:1992 +def fsplit_machine_functions : Flag <["-"], "fsplit-machine-functions">, + Group,

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

snehasish updated this revision to Diff 291063. snehasish added a comment. Check warning, specify target to avoid failures on windows. - Added a check for warning emitted if no profile is provided. - Added a triple for the remaining checks since we now emit an error for incompatible targets.

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

snehasish updated this revision to Diff 291051. snehasish added a comment. Updated test and warning type. - Updated test to check for the diagnostic message. - Updated BackendUtil.cpp to use backend optimization failure warning. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

snehasish updated this revision to Diff 290852. snehasish added a comment. Add a check for x86-elf. - Add a check for the triple in the driver to only enable the flag on x86+elf targets. - Add a test to ensure we don't enable this on other targets. Repository: rG LLVM Github Monorepo

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

tmsriram accepted this revision. tmsriram added a comment. This revision is now accepted and ready to land. This LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87047/new/ https://reviews.llvm.org/D87047

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

snehasish added a comment. Thanks for the comments! PTAL. In D87047#2252982 , @davidxl wrote: > For x86 target, should it be turned on when -fprofile-use= option is > specified unless -fno-split-machine-function is specified? I don't think we are there

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

snehasish updated this revision to Diff 289805. snehasish added a comment. Add warning when option is enabled without profile. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87047/new/ https://reviews.llvm.org/D87047 Files:

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

davidxl added a comment. For x86 target, should it be turned on when -fprofile-use= option is specified unless -fno-split-machine-function is specified? Also is it worth to give a warning if the option is specified but PGO is not on? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

snehasish created this revision. snehasish added reviewers: tmsriram, davidxl. Herald added subscribers: cfe-commits, dang. Herald added a project: clang. snehasish requested review of this revision. This patch adds a command line flag for the machine function splitter (added in rG94faadaca4e1