peter.smith added inline comments.
================
Comment at: clang/include/clang/Driver/Options.td:4162
def print_multi_lib : Flag<["-", "--"], "print-multi-lib">;
+def print_multi_selection_flags : Flag<["-", "--"],
"print-multi-selection-flags-experimental">,
+ HelpText<"Print the flags used for selecting multilibs (experimental)">;
----------------
any particular reason for using multi rather than multilib? Is there any intent
to use this anywhere else?
================
Comment at: clang/include/clang/Driver/ToolChain.h:289
+ /// Get flags suitable for multilib selection, based on the provided clang
+ /// command line arguments. The arguments aren't suitable to be used directly
+ /// for multilib selection because they are not normalized and normalization
----------------
Could be worth using "The command line arguments ..." when I first read I
didn't pick up on the distinction between flags and arguments. IIUC this
function translates command line arguments into normalised multilib selection
flags.
================
Comment at: clang/include/clang/Driver/ToolChain.h:303
+ /// returned separately. There may not be valid syntax to express this as a
+ /// clang argument so an pseudo argument syntax must be used instead.
+ /// To allow users to find out what flags are returned, clang accepts a
----------------
typo "a pseudo"
================
Comment at: clang/lib/Driver/ToolChain.cpp:173
}
+std::vector<std::string>
----------------
IIUC this needs to contain the union of all command-line options used by all
clients of the YAML but not any of the existing hard-coded Multilibs?
There is a possibility that this function could get large over time, with many
Toolchain and Architecture specific flags? It is it worth delegating some of
the work to the Toolchain, for example at the moment the BareMetal driver won't
support the sanitizers so this could be processed by a Toolchain specific
function (Fuchsia in this case?). There's likely to be a lot of options used by
many Multilib implementations so there is still scope for putting stuff here.
We would need to know what driver and architecture we are using here though.
If we had already constructed the MultilibSet it could be possible to filter
the flags against what the MultilibSet actually accepts?
Is it worth making that clear, or have some way for the other non YAML users to
call print_multi_selection_flags? Or is the option purely for
developing/debugging multilib as I'd not expect the output of
print_multi_section flags to be used by a user of clang. In that case a
translation of what command-line flags influence multilib selection (ideally
filtered) through the processed YAML file would be great.
================
Comment at: clang/lib/Driver/ToolChain.cpp:220
+
+ switch (Triple.getArch()) {
+ case llvm::Triple::arm:
----------------
Even though it might still live in this file, might be worth having all options
for a particular arch in a separate function. For example:
```
case llvm::Triple::arm:
case llvm::Triple::armeb:
case llvm::Triple::thumb:
case llvm::Triple::thumbeb:
addArmArchFlags(); // not put any thought into parameters etc.
```
================
Comment at: clang/lib/Driver/ToolChain.cpp:225
+ case llvm::Triple::thumbeb: {
+ std::vector<StringRef> Features;
+ unsigned FPUKind = llvm::ARM::FK_INVALID;
----------------
Can we handle the A profile situation where effectively we have v8 + a list of
features, with each v8.x and v9.y enabling a set of features
https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html
Ideally we would want to be able to do things like map v8.5-a+sve+sve2 and v9
to the same libraries.
================
Comment at: clang/lib/Driver/ToolChain.cpp:244
+#include "llvm/TargetParser/ARMTargetParser.def"
+ default:
+ llvm_unreachable("Invalid FPUKind");
----------------
I'm wondering if instead of using the FPU name we use a string representation
of the properties for the flags. For example FK_VFPV3_D16 this would be akin to
expanding march=armv8.6 to a list of target features. We'd need to make a
stable string representation but I think that should be possible.
================
Comment at: clang/test/Driver/print-multi-selection-flags.c:40
+// CHECK-MVE: target=thumbv8.1m.main-none-unknown-eabihf
+
+// RUN: %clang -print-multi-selection-flags-experimental
--target=arm-none-eabi -march=armv8.1m.main+mve+nofp | FileCheck
--check-prefix=CHECK-MVENOFP %s
----------------
Would be useful to have some Armv7-A and Armv8-A tests as well. In particular
I'm thinking of Arm-v8 and Arm-v8.1 as LSE atomics appear in v8.1.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D142933/new/
https://reviews.llvm.org/D142933
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
