chill added inline comments.
================
Comment at: clang/lib/Basic/Targets/ARM.cpp:387
+
+ return a.isArmT32();
+}
----------------
For example Arm7-a defines the T32 instruction set, buy we still want a warning.
Maybe we need `return a.isArmT32() && a.isArmMClass()`.
I'm not actually sure whether the triple correctly reflects the target
instruction set, e.g.
what are we going to get from `-target arm-eabi -march=armv7-a -mthumb`, so the
approach with
the target triple might not be working.
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:6402
+ CGM.getLangOpts().hasSignReturnAddress() ||
+ CGM.getLangOpts().isSignReturnAddressScopeAll()) {
+ // If the Branch Protection attribute is missing, validate the target
----------------
This condition `CGM.getLangOpts().isSignReturnAddressScopeAll()` is redundant.
================
Comment at: llvm/include/llvm/ADT/Triple.h:724
+ /// Tests whether the target is T32.
+ bool isArmT32() const {
----------------
This function does not look quite as expected.
`!isARM()` might be `isThumb()` but we're going to return false, isn't it ?
Then `isThumb()` might be true while we have, say, `armv6k`.
AFAICT, the test (and probably the whole function) ought to be
```
switch (auto SubArch = getSubArch()) {
case Triple::ARMSubArch_v8m_baseline,
case Triple::ARMSubArch_v7s:
case Triple::ARMSubArch_v7k:
case Triple::ARMSubArch_v7ve:
case Triple::ARMSubArch_v6:
case Triple::ARMSubArch_v6m:
case Triple::ARMSubArch_v6k:
case Triple::ARMSubArch_v6t2:
case Triple::ARMSubArch_v5:
case Triple::ARMSubArch_v5te:
case Triple::ARMSubArch_v4t:
return false;
default:
return true;
}
```
which is pretty future-proof.
================
Comment at: llvm/include/llvm/ADT/Triple.h:725
+ /// Tests whether the target is T32.
+ bool isArmT32() const {
+ if (!isARM())
----------------
In any case, if we're going to change the `Triple`, it should come with unit
tests in `unittest/ADT/TripleTest.cpp`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115501/new/
https://reviews.llvm.org/D115501
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
