simon_tatham added inline comments.
================
Comment at: clang/lib/Basic/Sanitizers.cpp:134-143
+ if ((A &&
+ A->getOption().matches(clang::driver::options::OPT_mexecute_only)) ||
+ (std::find(Features.begin(), Features.end(), "+execute-only") !=
+ Features.end())) {
+ // The execute-only output is supported only on ARMv6T2 and ARMv7 and
above.
+ if (llvm::ARM::parseArchVersion(Triple.getArchName()) > 7 ||
+ llvm::ARM::parseArch(Triple.getArchName()) ==
----------------
Why do we need to check //both// of `-mexecute-only` and the `+execute-only`
target feature? It looks to me as if the code in
`Driver/ToolChains/Arch/ARM.cpp` that handles `-mexecute-only` ends up setting
the same target feature anyway. And it checks the supported architecture
versions first.
Would it not be better to //just// check the target feature here, and avoid
having a duplicated copy of the rest of this logic which needs to be kept in
sync with the ARM driver?
Does something go wrong if you do it that way?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158614/new/
https://reviews.llvm.org/D158614
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
