[PATCH] D68049: Propeller: Clang options for basic block sections

This revision was automatically updated to reflect the committed changes. tmsriram marked 2 inline comments as done. Closed by commit rGe0bca46b0854: Options for Basic Block Sections, enabled in D68063 and D73674. (authored by tmsriram). Changed prior to commit:

[PATCH] D68049: Propeller: Clang options for basic block sections

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:964 +<< Opts.BBSections; + } + grimar wrote: > Doesn't seem you need "{}" around this

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram marked an inline comment as done. tmsriram added a comment. In D68049#2066937 , @MaskRay wrote: > LLD side changes look good. Are you waiting on an explicit approval from > @rmisth ? Yes. Comment at: lld/ELF/LTO.cpp:79

[PATCH] D68049: Propeller: Clang options for basic block sections

MaskRay added a comment. LLD side changes look good. Are you waiting on an explicit approval from @rmisth ? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 ___ cfe-commits mailing list

[PATCH] D68049: Propeller: Clang options for basic block sections

rsmith added a comment. Clang side of these changes LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D68049: Propeller: Clang options for basic block sections

MaskRay added inline comments. Comment at: lld/ELF/LTO.cpp:79 // Check if basic block sections must be used. // Allowed values for --lto-basicblock-sections are "all", "labels", // "", or none. This is the equivalent `--lto-basic-block-sections` I

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:1336 + +Generate labels for each basic block or place each basic block or a subset of basic blocks in its own section + rsmith wrote: > This file is automatically generated

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram updated this revision to Diff 264733. tmsriram marked 4 inline comments as done. tmsriram added a comment. Address reviewer comments: - New diagnostic kind - Document list= - Add docbrief CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049

[PATCH] D68049: Propeller: Clang options for basic block sections

rsmith added a comment. Clang side LG with some minor changes. Comment at: clang/docs/ClangCommandLineReference.rst:1336 + +Generate labels for each basic block or place each basic block or a subset of basic blocks in its own section + This file is

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram added a comment. Ping. Comment at: clang/include/clang/Basic/CodeGenOptions.h:114-127 + // -fbasic-block-sections=. The allowed values with this option are: + // {"labels", "all", "", "none"}. + // + // "labels": Only generate basic block symbols (labels) for

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram updated this revision to Diff 263371. tmsriram marked 5 inline comments as done and an inline comment as not done. tmsriram added a comment. Address reviewer comments: 1. Use Diagnostic instead of errs 2. Move test input to Inputs/ 3. Fix option description. CHANGES SINCE LAST ACTION

[PATCH] D68049: Propeller: Clang options for basic block sections

rsmith marked an inline comment as done. rsmith added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.h:114-127 + // -fbasic-block-sections=. The allowed values with this option are: + // {"labels", "all", "", "none"}. + // + // "labels": Only

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram marked an inline comment as done. tmsriram added inline comments. Comment at: lld/ELF/LTO.cpp:80 // Check if basic block sections must be used. // Allowed values for --lto-basicblock-sections are "all", "labels", // "", or none. This is the equivalent

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram updated this revision to Diff 263322. tmsriram added a comment. Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 Files: clang/docs/ClangCommandLineReference.rst clang/docs/UsersManual.rst

[PATCH] D68049: Propeller: Clang options for basic block sections

shenhan added inline comments. Comment at: lld/ELF/LTO.cpp:80 // Check if basic block sections must be used. // Allowed values for --lto-basicblock-sections are "all", "labels", // "", or none. This is the equivalent Maybe "--lto-basicblock-sections"

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram added a comment. @rsmith Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 ___ cfe-commits mailing list

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:486-501 + Options.BBSections = + llvm::StringSwitch(CodeGenOpts.BBSections) + .Case("all", llvm::BasicBlockSection::All) + .Case("labels", llvm::BasicBlockSection::Labels) +

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram updated this revision to Diff 259626. tmsriram marked 5 inline comments as done. tmsriram added a comment. Herald added subscribers: dexonsmith, steven_wu, hiraditya, arichardson, emaste. Herald added a reviewer: espindola. Document the flags, rename the options. CHANGES SINCE LAST

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram marked an inline comment as done. tmsriram added inline comments. Comment at: clang/include/clang/Driver/Options.td:1975 +def fbasicblock_sections_EQ : Joined<["-"], "fbasicblock-sections=">, Group, Flags<[CC1Option, CC1AsOption]>, + HelpText<"Place each function's

[PATCH] D68049: Propeller: Clang options for basic block sections

rsmith added a comment. Please add documentation for the new flags. Comment at: clang/include/clang/Driver/Options.td:1974 def fno_function_sections : Flag<["-"], "fno-function-sections">, Group; +def fbasicblock_sections_EQ : Joined<["-"], "fbasicblock-sections=">, Group,

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram added a comment. In D68049#1972297 , @dblaikie wrote: > In D68049#1971276 , @MaskRay wrote: > > > In D68049#1970825 , @tmsriram > > wrote: > > > > > Ping. > > > >

[PATCH] D68049: Propeller: Clang options for basic block sections

dblaikie added a comment. In D68049#1971276 , @MaskRay wrote: > In D68049#1970825 , @tmsriram wrote: > > > Ping. > > > @rsmith ^^^ > > More specific question, do you think >

[PATCH] D68049: Propeller: Clang options for basic block sections

MaskRay added a comment. In D68049#1970825 , @tmsriram wrote: > Ping. @rsmith ^^^ More specific question, do you think `clang/test/CodeGen/basicblock-sections.c` should be converted to a `-S -emit-llvm` test? CHANGES SINCE LAST ACTION

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram added inline comments. Comment at: clang/test/CodeGen/basicblock-sections.c:35 +// +// BB_WORLD: .section .text.world,"ax",@progbits +// BB_WORLD: world MaskRay wrote: > I haven't read through the previous threads whether we should use a .c -> .s >

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram updated this revision to Diff 254999. tmsriram marked 4 inline comments as done. tmsriram added a comment. Fix test and make error check at driver. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 Files:

[PATCH] D68049: Propeller: Clang options for basic block sections

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4858 + if (Arg *A = Args.getLastArg(options::OPT_fbasicblock_sections_EQ)) { +CmdArgs.push_back( If we want to pass the option verbatim, `A->render(Args, CmdArgs);` However,

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram updated this revision to Diff 253271. tmsriram marked 5 inline comments as done. tmsriram added a reviewer: eli.friedman. tmsriram added a comment. Clang options for Basic Block Sections enabled in D68063 and D73674 1.

[PATCH] D68049: Propeller: Clang options for basic block sections

lebedev.ri added a comment. In D68049#1870401 , @tmsriram wrote: > In D68049#1870094 , @tmsriram wrote: > > > In D68049#1868623 , @MaskRay wrote: > > > > > > In

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram added a comment. >>> I think the patch series should probably be structured this way: >>> >>> 1. LLVM CodeGen: enables basic block sections. >>> 2. clang Driver/Frontend/CodeGen: pass basic block sections options to LLVM. >>> 3. LLVM CodeGen: which enables the rest of Propeller options.

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram added a comment. In D68049#1870094 , @tmsriram wrote: > In D68049#1868623 , @MaskRay wrote: > > > > In D68049#1865967 , @MaskRay > > > wrote: > > > If you don't

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram updated this revision to Diff 243947. tmsriram added a comment. Remove usage of "propeller". Fix header inclusion. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 Files: clang/include/clang/Basic/CodeGenOptions.def

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram added a comment. In D68049#1870094 , @tmsriram wrote: > In D68049#1868623 , @MaskRay wrote: > > > > In D68049#1865967 , @MaskRay > > > wrote: > > > If you don't

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram added a comment. In D68049#1868623 , @MaskRay wrote: > > In D68049#1865967 , @MaskRay wrote: > > If you don't mind, I can push a Diff to this Differential which will > > address these review comments. >

[PATCH] D68049: Propeller: Clang options for basic block sections

MaskRay added a comment. > In D68049#1865967 , @MaskRay wrote: > If you don't mind, I can push a Diff to this Differential which will address > these review comments. I can't because I can't figure out the patch relationship... First, this patch does

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram updated this revision to Diff 243614. tmsriram marked 3 inline comments as done. tmsriram added a comment. Removed getBBSectionsList (moved to LLVM) and address other reviewer comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram added a comment. In D68049#1865967 , @MaskRay wrote: > If you don't mind, I can push a Diff to this Differential which will address > these review comments. Let me update this patch asap as we refactored getBBSectionsList into llvm as it is

[PATCH] D68049: Propeller: Clang options for basic block sections

grimar added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.h:120 + std::string BasicBlockSections; + MaskRay wrote: > Comment its allowed values ("all", "labels", "none") I'd suggest to rewrite it somehow. This set of values did not

[PATCH] D68049: Propeller: Clang options for basic block sections

MaskRay added a comment. If you don't mind, I can push a Diff to this Differential which will address these review comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:436 + + std::ifstream fin(FunctionsListFile); + if (!fin.good()) {

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram updated this revision to Diff 242196. tmsriram added a comment. Splitting the clang patch into several pieces: 1. This is the parent patch and just contains support for basic block section options. 2. A separate patch for unique internal function names 3. A separate patch for an option

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram updated this revision to Diff 238620. tmsriram added a comment. clang-formatted. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 Files: clang/include/clang/Basic/CodeGenOptions.def clang/include/clang/Basic/CodeGenOptions.h

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram updated this revision to Diff 238552. tmsriram added a comment. Updated to top of trunk, bug fixes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 Files: clang/include/clang/Basic/CodeGenOptions.def

[PATCH] D68049: Propeller: Clang options for basic block sections

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:631 +if (A->getOption().matches(options::OPT_fpropeller_optimize_EQ)) { + if (!Args.getLastArgValue(options::OPT_fuse_ld_EQ).equals_lower("lld")) +

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram marked 13 inline comments as done. tmsriram added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.def:341 +CODEGENOPT(RelocateWithSymbols, 1, 0) + /// Whether we should use the undefined behaviour optimization for control flow

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram updated this revision to Diff 222035. tmsriram added a comment. Updated patch to address the comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 Files: clang/include/clang/Basic/CodeGenOptions.def

[PATCH] D68049: Propeller: Clang options for basic block sections

MaskRay added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.h:120 + std::string BasicBlockSections; + Comment its allowed values ("all", "labels", "none") Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1103 +

[PATCH] D68049: Propeller: Clang options for basic block sections

MaskRay added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:439 +// Empty '!' implies no more functions. +if (S.size() == 1 && S[0] == '!') + break; ``` if (S.consume_front("!")) { if (S.empty()) ... else ... } ```

[PATCH] D68049: Propeller: Clang options for basic block sections

mehdi_amini added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.def:52 + ///< Produce unique section names with + ///< basic block sections. ENUM_CODEGENOPT(FramePointer,

[PATCH] D68049: Propeller: Clang options for basic block sections

tmsriram created this revision. tmsriram added a reviewer: rnk. Herald added a project: clang. Options for basic block sections, unique internal linkage function names. This is part of the Propeller framework to do post link code layout optimizations. Please see the RFC here: