[clang][Serialization][RISCV] Increase the number of reserved predefined type IDs

[rogfer01]

Summary

In D152070 we added many new intrinsic types required for the RISC-V Vector Extension.

This was crashing when loading the AST as those types are intrinsically added to the AST (they don't come from the disk).

$ touch t.c
$ clang -cc1 -triple riscv64 -w -emit-pch -o test.pch t.c
$ clang -cc1 -triple riscv64 -w -x c -include-pch test.pch -ast-dump-all /dev/null

The total number required now by clang goes far beyond 300 so increasing the value to 500 solves the problem. This value was already increased in D92715 but I assume this has some impact on the on-disk format.

[rogfer01]

Let me check if I can add a static_assert to get a compile-time failure.

[rogfer01]

ChangeLog:

• Add a sentinel and a static_assert

[eopXD]

This change looks good to me. Weird that the error did not pop out on my side.

[zixuan-wu]

I also caught this issue.

[rogfer01]

Thanks @eopXD. Not sure why you didn't observe the failure. I understand your build has -DLLVM_ENABLE_ASSERTIONS=ON.

I investigated a bit the libcxx errors flagged by the precommit CI. I can get similar (but not exactly the same errors) if the build before this change is -DLLVM_ENABLE_PROJECTS=clang -DLLVM_ENABLE_RUNTIMES=libcxx, then I apply this change and then I run libcxx tests: several precompiled headers become stale after this change and would need to be rebuilt. Starting from a clean build directory avoids this issue.

I want to do a few more checks locally before committing. Sorry for the delay.

[rogfer01]

ChangeLog:

• Update clang tests that now, on RISC-V only, observe a larger precompiled module file

clang/include/clang/Serialization/ASTBitCodes.h

 #define PPC_VECTOR_TYPE(Name, Id, Size) PREDEF_TYPE_##Id##_ID,
 #include "clang/Basic/PPCTypes.def"
 // \brief RISC-V V types with auto numeration
 #define RVV_TYPE(Name, Id, SingletonId) PREDEF_TYPE_##Id##_ID,
 #include "clang/Basic/RISCVVTypes.def"
 // \brief WebAssembly reference types with auto numeration
 #define WASM_TYPE(Name, Id, SingletonId) PREDEF_TYPE_##Id##_ID,
 #include "clang/Basic/WebAssemblyReferenceTypes.def"
+  // Sentinel value. Considered a predefined type but not useable as one.
+  PREDEF_TYPE_LAST_ID
 };
 
 /// The number of predefined type IDs that are reserved for
 /// the PREDEF_TYPE_* constants.
 ///
 /// Type IDs for non-predefined types will start at
 /// NUM_PREDEF_TYPE_IDs.
-const unsigned NUM_PREDEF_TYPE_IDS = 300;
+const unsigned NUM_PREDEF_TYPE_IDS = 500;
+
+// Ensure we do not overrun the predefined types we reserved
+// in the enum PredefinedTypeIDs above.
+static_assert(PREDEF_TYPE_LAST_ID < NUM_PREDEF_TYPE_IDS,
+              "Too many enumerators in PredefinedTypeIDs. Review the value of "
+              "NUM_PREDEF_TYPE_IDS");
 
 /// Record codes for each kind of type.
 ///
 /// These constants describe the type records that can occur within a
 /// block identified by DECLTYPES_BLOCK_ID in the AST file. Each
 /// constant describes a record for a specific type class in the
 /// AST. Note that DeclCode values share this code space.
 enum TypeCode {

clang/lib/Serialization/ASTReader.cpp

QualType ASTReader::GetType(TypeID ID) {
   ASTContext &Context = *ContextObj;
 
   unsigned FastQuals = ID & Qualifiers::FastMask;
   unsigned Index = ID >> Qualifiers::FastWidth;
 
   if (Index < NUM_PREDEF_TYPE_IDS) {
     QualType T;
     switch ((PredefinedTypeIDs)Index) {
+    case PREDEF_TYPE_LAST_ID:
+      // We should never use this one.
+      llvm_unreachable("Invalid predefined type");
+      break;
     case PREDEF_TYPE_NULL_ID:
       return QualType();
     case PREDEF_TYPE_VOID_ID:
       T = Context.VoidTy;
       break;
     case PREDEF_TYPE_BOOL_ID:
       T = Context.BoolTy;
       break;

clang/test/Modules/embed-files-compressed.cpp

 // RUN: cat %t/b.h %t/b.h %t/b.h %t/b.h > %t/a.h
 // RUN: cat %t/a.h %t/a.h %t/a.h %t/a.h > %t/b.h
 // RUN: cat %t/b.h %t/b.h %t/b.h %t/b.h > %t/a.h
 // RUN: echo 'module a { header "a.h" }' > %t/modulemap
 //
 // RUN: %clang_cc1 -fmodules -I%t -fmodules-cache-path=%t -fmodule-name=a -emit-module %t/modulemap -fmodules-embed-all-files -o %t/a.pcm
 //
 // The above embeds ~4.5MB of highly-predictable /s and \ns into the pcm file.
-// Check that the resulting file is under 40KB:
+// Check that the resulting file is under 60KB:
 //
 // RUN: wc -c %t/a.pcm | FileCheck --check-prefix=CHECK-SIZE %s
-// CHECK-SIZE: {{(^|[^0-9])[123][0-9][0-9][0-9][0-9]($|[^0-9])}}
+// CHECK-SIZE: {{(^|[^0-9])[1-5][0-9][0-9][0-9][0-9]($|[^0-9])}}

clang/test/Modules/empty.modulemap

 // RUN: rm -rf %t
 //
 // RUN: %clang_cc1 -fmodules -x c++ -fmodules-cache-path=%t \
 // RUN:   -fno-implicit-modules \
 // RUN:   -emit-module -fmodule-name=empty -o %t/base.pcm \
 // RUN:   %s
 //
 // RUN: %clang_cc1 -fmodules -x c++ -fmodules-cache-path=%t \
 // RUN:   -fno-implicit-modules \
 // RUN:   -emit-module -fmodule-name=empty -o %t/check.pcm \
 // RUN:   %s
 //
 // The module file should be identical each time we produce it.
 // RUN: diff %t/base.pcm %t/check.pcm
 //
-// We expect an empty module to be less than 40KB (and at least 10K, for now).
+// We expect an empty module to be less than 60KB (and at least 10K, for now).
 // RUN: wc -c %t/base.pcm | FileCheck --check-prefix=CHECK-SIZE %s
-// CHECK-SIZE: {{(^|[^0-9])[123][0-9][0-9][0-9][0-9]($|[^0-9])}}
+// CHECK-SIZE: {{(^|[^0-9])[1-5][0-9][0-9][0-9][0-9]($|[^0-9])}}
 
 module empty { header "Inputs/empty.h" export * }