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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
