[clang] ed86058 - Add static assert to ID Table to make sure aux targets work right.

Erich Keane via cfe-commits cfe-commits at lists.llvm.org
Thu May 7 12:50:00 PDT 2020


Author: Erich Keane
Date: 2020-05-07T12:49:46-07:00
New Revision: ed86058b53f971ed93cc79c8b4fc76da37ca0664

URL: https://github.com/llvm/llvm-project/commit/ed86058b53f971ed93cc79c8b4fc76da37ca0664
DIFF: https://github.com/llvm/llvm-project/commit/ed86058b53f971ed93cc79c8b4fc76da37ca0664.diff

LOG: Add static assert to ID Table to make sure aux targets work right.

I discovered that the limit on possible builtins managed by this
ObjCOrBuiltin variable is too low when combining large targets, since
aux-targets are appended to the targets list. A runtime assert exists
for this, however this patch creates a static-assert as well.

The logic for said static-assert is to make sure we have the room for
the aux-target and target to both be the largest list, which makes sure
we have room for all possible combinations.

I also incremented the number of bits by 1, since I discovered this
currently broken.  The current bit-count was 36, so this doesn't
increase any size.

Added: 
    

Modified: 
    clang/include/clang/Basic/IdentifierTable.h
    clang/include/clang/Basic/TargetBuiltins.h
    clang/lib/Basic/IdentifierTable.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/IdentifierTable.h b/clang/include/clang/Basic/IdentifierTable.h
index f23e7276b030..31849bbdd545 100644
--- a/clang/include/clang/Basic/IdentifierTable.h
+++ b/clang/include/clang/Basic/IdentifierTable.h
@@ -48,6 +48,8 @@ using IdentifierLocPair = std::pair<IdentifierInfo *, SourceLocation>;
 /// of a pointer to one of these classes.
 enum { IdentifierInfoAlignment = 8 };
 
+static constexpr int ObjCOrBuiltinIDBits = 14;
+
 /// One of these records is kept for each identifier that
 /// is lexed.  This contains information about whether the token was \#define'd,
 /// is a language keyword, or if it is a front-end token of some sort (e.g. a
@@ -63,7 +65,7 @@ class alignas(IdentifierInfoAlignment) IdentifierInfo {
   // ObjC keyword ('protocol' in '@protocol') or builtin (__builtin_inf).
   // First NUM_OBJC_KEYWORDS values are for Objective-C,
   // the remaining values are for builtins.
-  unsigned ObjCOrBuiltinID : 13;
+  unsigned ObjCOrBuiltinID : ObjCOrBuiltinIDBits;
 
   // True if there is a #define for this.
   unsigned HasMacro : 1;

diff  --git a/clang/include/clang/Basic/TargetBuiltins.h b/clang/include/clang/Basic/TargetBuiltins.h
index bf07a8950f28..db10077895ee 100644
--- a/clang/include/clang/Basic/TargetBuiltins.h
+++ b/clang/include/clang/Basic/TargetBuiltins.h
@@ -15,6 +15,7 @@
 #ifndef LLVM_CLANG_BASIC_TARGETBUILTINS_H
 #define LLVM_CLANG_BASIC_TARGETBUILTINS_H
 
+#include <algorithm>
 #include <stdint.h>
 #include "clang/Basic/Builtins.h"
 #include "llvm/Support/MathExtras.h"
@@ -308,6 +309,14 @@ namespace clang {
     };
   }
 
+  static constexpr uint64_t LargestBuiltinID = std::max<uint64_t>(
+      {NEON::FirstTSBuiltin, ARM::LastTSBuiltin, SVE::FirstTSBuiltin,
+       AArch64::LastTSBuiltin, BPF::LastTSBuiltin, PPC::LastTSBuiltin,
+       NVPTX::LastTSBuiltin, AMDGPU::LastTSBuiltin, X86::LastTSBuiltin,
+       Hexagon::LastTSBuiltin, Mips::LastTSBuiltin, XCore::LastTSBuiltin,
+       Le64::LastTSBuiltin, SystemZ::LastTSBuiltin,
+       WebAssembly::LastTSBuiltin});
+
 } // end namespace clang.
 
 #endif

diff  --git a/clang/lib/Basic/IdentifierTable.cpp b/clang/lib/Basic/IdentifierTable.cpp
index d7ef159743b0..36b26d9b7c68 100644
--- a/clang/lib/Basic/IdentifierTable.cpp
+++ b/clang/lib/Basic/IdentifierTable.cpp
@@ -16,6 +16,7 @@
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/Specifiers.h"
+#include "clang/Basic/TargetBuiltins.h"
 #include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/ADT/FoldingSet.h"
@@ -32,6 +33,12 @@
 
 using namespace clang;
 
+// A check to make sure the ObjCOrBuiltinID has sufficient room to store the
+// largest possible target/aux-target combination. If we exceed this, we likely
+// need to just change the ObjCOrBuiltinIDBits value in IdentifierTable.h.
+static_assert(2 * LargestBuiltinID < (2 << (ObjCOrBuiltinIDBits - 1)),
+              "Insufficient ObjCOrBuiltinID Bits");
+
 //===----------------------------------------------------------------------===//
 // IdentifierTable Implementation
 //===----------------------------------------------------------------------===//


        


More information about the cfe-commits mailing list