peter.smith added a comment.
Looks good to me. Some small suggestions around execute-only as I expect this
to be a useful multilib variant.
================
Comment at: clang/lib/Driver/ToolChain.cpp:198
unsigned FPUKind = llvm::ARM::FK_INVALID;
- tools::arm::getARMTargetFeatures(D, Triple, Args, Features, false, &FPUKind);
+ tools::arm::getARMTargetFeatures(D, Triple, Args, Features, false, false,
+ &FPUKind);
----------------
Now that there are two boolean parameters, could you put a prefix comment to
show which one is which for example:
/* ForAs */ false, /* GenExecuteOnly */
================
Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:779
// This only makes sense for the compiler, not for the assembler.
- if (!ForAS) {
+ if (!ForAS && GetExecuteOnly) {
// Supported only on ARMv6T2 and ARMv7 and above.
----------------
I can see execute-only being used in a multilib configuration as it can impact
code-generation significantly yet is a requirement for a system aiming to have
all code execute-only.
Rather than a specific `GenExecuteOnly` flag, would it be better for something
like `ForMultilib` like `ForAS` and then we could move it to not give the
error message when doing `ForMultilib` the name would also generalise to other
parts.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D142986/new/
https://reviews.llvm.org/D142986
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
