[PATCH] D72227: Add options for clang to align branches within 32B boundary

skan marked an inline comment as done and an inline comment as not done. skan added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2045 + + if (Args.hasFlag(options::OPT_mbranches_within_32B_boundaries, + options::OPT_mno_branches_within_32B

[PATCH] D72227: Add options for clang to align branches within 32B boundary

skan updated this revision to Diff 237245. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72227/new/ https://reviews.llvm.org/D72227 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Clang.cpp clang/test/Driver/x86-malign-branch.c Index: clang/test/Driver/x86

[PATCH] D72227: Add options for clang to align branches within 32B boundary

skan marked an inline comment as done. skan added inline comments. Comment at: clang/include/clang/Driver/Options.td:2200 + "Control how the assembler should align branches with NOP or segment " + "override prefix. If the boundary's size is not 0, it should be

[PATCH] D72227: Add options for clang to align branches within 32B boundary

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2045 + + if (Args.hasFlag(options::OPT_mbranches_within_32B_boundaries, + options::OPT_mno_branches_within_32B_boundaries, false)) { LuoYuanke wrote: > MaskRay wro

[PATCH] D72227: Add options for clang to align branches within 32B boundary

LuoYuanke added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2045 + + if (Args.hasFlag(options::OPT_mbranches_within_32B_boundaries, + options::OPT_mno_branches_within_32B_boundaries, false)) { MaskRay wrote: > skan wrote:

[PATCH] D72227: Add options for clang to align branches within 32B boundary

MaskRay added a comment. Created D72463 (`-mbranches-within-32B-boundaries` can be overridden by `-malign-branch*`; added test coverage) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72227/new/ https://reviews.llvm.org/D72227 __

[PATCH] D72227: Add options for clang to align branches within 32B boundary

craig.topper added inline comments. Comment at: clang/include/clang/Driver/Options.td:2196 +: Joined<["-"], "malign-branch-boundary=">, + Group, + Flags<[DriverOption, HelpHidden]>, MaskRay wrote: > `Group` Doesn't m_x86_Features_Group make things g

[PATCH] D72227: Add options for clang to align branches within 32B boundary

skan updated this revision to Diff 237019. skan marked 5 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72227/new/ https://reviews.llvm.org/D72227 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Clang.cpp clang/test/Driver/intel-alig

[PATCH] D72227: Add options for clang to align branches within 32B boundary

MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:2196 +: Joined<["-"], "malign-branch-boundary=">, + Group, + Flags<[DriverOption, HelpHidden]>, `Group` Comment at: clang/include/clang/Driver/Option

[PATCH] D72227: Add options for clang to align branches within 32B boundary

skan updated this revision to Diff 236945. skan marked an inline comment as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72227/new/ https://reviews.llvm.org/D72227 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Clang.cpp clang/test/Driver/intel-alig

[PATCH] D72227: Add options for clang to align branches within 32B boundary

MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:2200 + "from being across or against the boundary of specified size. " + "-x86-align-branch-boundary=0 doesn't align branches.">; +def malign_branch_EQ > -x86-align

[PATCH] D72227: Add options for clang to align branches within 32B boundary

skan marked 5 inline comments as done. skan added inline comments. Comment at: clang/include/clang/Driver/Options.td:2217 + HelpText<"Specify the maximum number of prefixes on an instruction to " + "align branches. The number should be between 0 and 4.">; +def

[PATCH] D72227: Add options for clang to align branches within 32B boundary

skan updated this revision to Diff 236547. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72227/new/ https://reviews.llvm.org/D72227 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Clang.cpp clang/test/Driver/intel-align-branch.c Index: clang/test/Driver/in

[PATCH] D72227: Add options for clang to align branches within 32B boundary

annita.zhang added inline comments. Comment at: clang/include/clang/Driver/Options.td:2198 + "should be a power of 2 and no less than 32. Branches will be " + "aligned within the boundary of specified size. " + "-x86-align-branch-boundary

[PATCH] D72227: Add options for clang to align branches within 32B boundary

annita.zhang added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2052 +CmdArgs.push_back("-mllvm"); +CmdArgs.push_back(Args.MakeArgString("-x86-align-branch-prefix-size=4")); + } CmdArgs.push_back(Args.MakeArgString("-x86-align-branc

[PATCH] D72227: Add options for clang to align branches within 32B boundary

annita.zhang requested changes to this revision. annita.zhang added a comment. This revision now requires changes to proceed. A general comment. All the tests have been done with -malign-branch-prefix-size=5. I don't see any explicit reason to change the default prefix size from 5 to 4. CHANGE

[PATCH] D72227: Add options for clang to align branches within 32B boundary

skan added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2012 + ArgStringList &CmdArgs) { + if (const Arg *A = Args.getLastArg(options::OPT_malign_branch_boundary_EQ)) { +StringRef Value = A->getValue(); M

[PATCH] D72227: Add options for clang to align branches within 32B boundary

skan updated this revision to Diff 236329. skan marked 5 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72227/new/ https://reviews.llvm.org/D72227 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Clang.cpp clang/test/Driver/intel-alig

[PATCH] D72227: Add options for clang to align branches within 32B boundary

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2010 +static void AlignBranchesOptions(const Driver &D, const ArgList &Args, + ArgStringList &CmdArgs) { The convention is `camelCase`. This file is

[PATCH] D72227: Add options for clang to align branches within 32B boundary

skan updated this revision to Diff 236280. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72227/new/ https://reviews.llvm.org/D72227 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Clang.cpp clang/test/Driver/intel-align-branch.c Index: clang/test/Driver/in

[PATCH] D72227: Add options for clang to align branches within 32B boundary

skan created this revision. skan added reviewers: craig.topper, LuoYuanke, annita.zhang, jyknight, reames, MaskRay. Herald added a project: clang. Herald added a subscriber: cfe-commits. Add an option -mbranches-within-32B-boundaries to align branches within a 32-Byte boundary to reduce the p