[PATCH] D142986: Enable multilib.yaml in the BareMetal ToolChain

[Peter Smith via Phabricator via cfe-commits]

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