[PATCH] D72463: [Driver][X86] Add -malign-branch* and -malign-branch-within-32B-boundaries

Fri Jan 10 03:10:59 PST 2020

skan added a comment.

In D72463#1813669 <https://reviews.llvm.org/D72463#1813669>, @MaskRay wrote:

> The commutative property (independence of options) makes option composition easier. clangDriver makes heavy use of `getLastArg` and `hasArg`. Without the commutative property, it would now be able to shuffle code around.
>
> The `-malign-branch=jmp -mno-branches-within-32B-boundaries` issue does not matter that much. I do not expect `-mno-branches-within-32B-boundaries` to be used much. If we want to disable branch alignment, `-malign-branch-boundary=0` can be used.

The behaviour of your proposal is "specific option is always override the gerneral option, no matter which is the last". I still prefer not to support the override, which makes things more clear. But I am fine if other reviewers believe this behaviour is reasonable. @jyknight  @reames @craig.topper

================
Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:254
+def err_drv_invalid_malign_branch_EQ : Error<
+  "invalid argument '%0' to -malign-branch=; must be one of: %1">;
+
----------------
The error information "must be one of: " is kind of misleading.
It seems that one kind of branch can be aligned.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72463