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 inconsistent but some newly added
functions follow this convention.
================
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();
----------------
if (!TC.getTriple().isX86()) {
D.Diag(diag::err_drv_unsupported_opt_for_target)
<< A->getAsString(Args) << TripleStr;
return;
}
I will add Triple::isX86 in D72247.
================
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)) {
----------------
`OPT_mbranches_within_32B_boundaries` should provide default values which can
be overridden by more specific options.
================
Comment at: clang/test/Driver/intel-align-branch.c:28
+//
+// // RUN: %clang -target x86_64-unknown-unknown
-mbranches-within-32B-boundaries -### -c %s 2>&1 | FileCheck %s
--check-prefix=CHECK-TOTAL
+// CHECK-TOTAL: "-mllvm" "-x86-align-branch-boundary=32" "-mllvm"
"-x86-align-branch=fused+jcc+jmp" "-mllvm" "-x86-align-branch-prefix-size=4"
----------------
Since you have a `-mno.. -m..` test, the `-m..` test is redundant.
================
Comment at: clang/test/Driver/intel-align-branch.c:35
+// RUN: %clang -target x86_64-unknown-unknown -mbranches-within-32B-boundaries
-mno-branches-within-32B-boundaries -### -c %s 2>&1 | FileCheck %s
--check-prefix=CHECK-TOTAL3
+// CHECK-TOTAL3-NOT: "-mllvm" "-x86-align-branch-boundary=32" "-mllvm"
"-x86-align-branch=fused+jcc+jmp" "-mllvm" "-x86-align-branch-prefix-size=4"
----------------
Add a `-target aarch64` test that other targets will get an error.
(I am lazy and usually just write `-target x86_64` (generic ELF) and omit
`-unknown-known`)
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
