[PATCH] D115501: [clang][ARM] Emit warnings when PACBTI-M is used with unsupported architectures

[Momchil Velikov via Phabricator via cfe-commits]

chill added inline comments.


================
Comment at: clang/lib/Basic/Targets/AArch64.h:70
 
-  bool validateBranchProtection(StringRef, BranchProtectionInfo &,
+  bool validateBranchProtection(StringRef, StringRef, BranchProtectionInfo &,
                                 StringRef &) const override;
----------------
Would be nice to have parameter names, like in the adjacent declarations.


================
Comment at: clang/lib/Basic/Targets/ARM.cpp:375-379
+  if (ArchKind != llvm::ARM::ArchKind::ARMV8_1MMainline &&
+      ArchKind != llvm::ARM::ArchKind::ARMV8MMainline &&
+      ArchKind != llvm::ARM::ArchKind::ARMV7M &&
+      ArchKind != llvm::ARM::ArchKind::ARMV7EM)
+    return false;
----------------



================
Comment at: clang/lib/Basic/Targets/ARM.cpp:381
+
+  return true;
+}
----------------



================
Comment at: clang/lib/Basic/Targets/ARM.cpp:391-392
 
+  if (!Arch.empty() && !isBranchProtectionSupportedArch(Arch))
+    return false;
+
----------------
On empty `Arch` it'd continue down the function, but we'd like to return failure.


================
Comment at: clang/lib/Basic/Targets/ARM.h:128
 
-  bool validateBranchProtection(StringRef, BranchProtectionInfo &,
+  bool isBranchProtectionSupportedArch(StringRef) const override;
+  bool validateBranchProtection(StringRef, StringRef, BranchProtectionInfo &,
----------------
Likewise.


================
Comment at: clang/test/CodeGen/arm_acle.c:1530
 // AArch64-NEXT:    ret i32 [[TMP0]]
 //
 uint32_t test_crc32cd(uint32_t a, uint64_t b) {
----------------
These look like random changes for the untrained eye


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115501/new/

https://reviews.llvm.org/D115501