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

[LuoYuanke via Phabricator via cfe-commits]

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:
> > MaskRay wrote:
> > > `OPT_mbranches_within_32B_boundaries` should provide default values which can be overridden by more specific options.
> > Currently, `-mbranches-within-32B-boundaries` is equivalent to `-malign-branch-boundary=32 -malign-branch=fused+jcc+jmp -malign-branch-prefix-size=4
> > 
> > What is expected behaviour would be very confusing if specific options could override `-mbranches-within-32B-boundaries`. For example, if passed options are
> > 
> > ```
> > -mbranches-within-32B-boundaries -malign-branch-boundary=32 -mno-branches-within-32B-boundaries
> > ```
> > What should the value of `-malign-branch-boundary` be?  Is it 32 or 0?
> > 
> > If we think `-mno-branches-within-32B-boundaries` is the negative form of `-mbranches-within-32B-boundaries` , then `-malign-branch-boundary` should be 32.
> > 
> > Or if we think `-mno-branches-within-32B-boundaries` wins since it appears at the end, and `-mno-branches-within-32B-boundaries` means no need to align branches, `-malign-branch-boundary` should be 0.
> > 
> > As long as we don't support specific options could override `-mbranches-within-32B-boundaries`, the trouble disappears  :-)
> > 
> > 
> > -mbranches-within-32B-boundaries -malign-branch-boundary=32 -mno-branches-within-32B-boundaries
> 
> My preference is that the net effect will be: `-malign-branch-boundary=32`
> 
> ```
> If (Args.hasFlag(options::OPT_mbranches_within_32B_boundaries, options::OPT_mno_branches_within_32B_boundaries, false))
>   boundary = 32;
> if (const Arg *A = Args.getLastArg(options::OPT_malign_branch_EQ))
>   boundary = ...
> if (boundary)
>   add -mllvm boundary
> ```
> 
> but I'd like to hear what others say. @jyknight @reames 
I have no preference. What's the general rule for such case in LLVM? Is there any similar option design before? 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72227/new/

https://reviews.llvm.org/D72227