[clang] Add serialization for loop hint annotation tokens

[mikerice]

Summary

When late parsed templates are used with PCH, tokens are serialized. The
existing code does not handle annotation tokens which can occur due to
various pragmas.

This patch implements the serialization for annot_pragma_loop_hint.

This also enables use of OpenMP pragmas and #pragma unused which do not
need special serialization of the PtrData field.

There are other pragmas that can be used with late template parsing which
we can implement in separately patches as needed.

Fixes https://github.com/llvm/llvm-project/issues/39504

[aaron.ballman]

I think you also need to update ASTBitCodes.h to bump VERSION_MAJOR, and the changes should have a release note.

clang/include/clang/Lex/Token.h

338

ArrayRef does not own its underlying data, so this looks like a potential use-after-free waiting to happen despite being existing code.

clang/lib/Serialization/ASTReader.cpp

1687

Ah, so you make an allocated copy of the tokens, so this isn't a use-after-free but it's still pretty fragile. I wonder if we should switch to using SmallVector in a follow-up NFC change?

clang/lib/Serialization/ASTWriter.cpp

4397–4398

There's a whole lot of annotation tokens that are not loop hints; is something preventing them from getting here?

[mikerice]

Bump the AST version.
Add an entry to the release notes.

clang/lib/Serialization/ASTReader.cpp

1687

I can look into that and follow-up. Not sure why that choice was made originally.

clang/lib/Serialization/ASTWriter.cpp

4397–4398

I believe the non-pragma annotation tokens are not created at this point since Sema hasn't been run over the late-parsed template. Annotation tokens for pragmas will hit this assertion until someone implements serialization for that pragma. This is a little better than the assertion we had before. Another option would be to give an error here if we see a token we haven't implemented yet.

[rnk]

It's unfortunate that nobody has carried forward an RFC to disable fdelayed-template-parsing:
https://discourse.llvm.org/t/how-do-you-access-the-body-of-a-template-function-in-the-ast/61829/7

This feature is a source of bugs and ongoing maintenance costs like this, and I don't believe it is creating enough value to justify itself anymore.

Feel free to do what you have to, though.

[mikerice]

ping, anything else needed for this?

[aaron.ballman]

In D138453#3944926, @rnk wrote:

It's unfortunate that nobody has carried forward an RFC to disable fdelayed-template-parsing:
https://discourse.llvm.org/t/how-do-you-access-the-body-of-a-template-function-in-the-ast/61829/7

This feature is a source of bugs and ongoing maintenance costs like this, and I don't believe it is creating enough value to justify itself anymore.

I think the compatibility it gives with MSVC is sufficient value to justify its existence (until MSVC stops supporting delayed template parsing entirely), but we should be exploring whether we have the correct default these days in terms of enabling/disabling it for targets.

In D138453#3957293, @mikerice wrote:

ping, anything else needed for this?

LGTM now, thanks!

[shafik]

It looks like https://github.com/llvm/llvm-project/issues/60543 is hitting your added llvm_unreachable("missing serialization code for annotation token"); is this expected.

[DHowett-MSFT]

In D138453#4105209, @shafik wrote:

It looks like https://github.com/llvm/llvm-project/issues/60543 is hitting your added llvm_unreachable("missing serialization code for annotation token"); is this expected.

I believe it's expected that we hit this assertion - it's clearer than the original message, and more directly indicates that #pragma pack cannot be serialized.

[aaron.ballman]

In D138453#4105211, @DHowett-MSFT wrote:

In D138453#4105209, @shafik wrote:

It looks like https://github.com/llvm/llvm-project/issues/60543 is hitting your added llvm_unreachable("missing serialization code for annotation token"); is this expected.

I believe it's expected that we hit this assertion - it's clearer than the original message, and more directly indicates that #pragma pack cannot be serialized.

Failed assertions or reaching code marked as unreachable is always a bug -- we should be failing gracefully with diagnostics and not crashing.

[mikerice]

In D138453#4106666, @aaron.ballman wrote:

In D138453#4105211, @DHowett-MSFT wrote:

I believe it's expected that we hit this assertion - it's clearer than the original message, and more directly indicates that #pragma pack cannot be serialized.

Failed assertions or reaching code marked as unreachable is always a bug -- we should be failing gracefully with diagnostics and not crashing.

My intention is to go through all the pragmas and implement serialization for any that need it. If there are some that we don't want to do or if this takes a long time, I will do ahead and add a diagnostic.

clang/docs/ReleaseNotes.rst

- If the environment variable ``SOURCE_DATE_EPOCH`` is set, it specifies a UNIX
   timestamp to be used in replacement of the current date and time in
   the ``__DATE__``, ``__TIME__``, and ``__TIMESTAMP__`` macros. See
   `<https://reproducible-builds.org/docs/source-date-epoch/>`_.
 - Clang now supports ``__has_constexpr_builtin`` function-like macro that
   evaluates to 1 if the builtin is supported and can be constant evaluated.
   It can be used to writing conditionally constexpr code that uses builtins.
 - The time profiler (using ``-ftime-trace`` option) now traces various constant
   evaluation events.
+- Clang can now generate a PCH when using ``-fdelayed-template-parsing`` for
+  code with templates containing loop hint pragmas, OpenMP pragmas, and
+  ``#pragma unused``.
+
 
 New Compiler Flags
 ------------------
 
 - Implemented `-fcoro-aligned-allocation` flag. This flag implements
   Option 2 of P2014R0 aligned allocation of coroutine frames
   (`P2014R0 <https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p2014r0.pdf>`_).
   With this flag, the coroutines will try to lookup aligned allocation

clang/include/clang/Lex/Token.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_LEX_TOKEN_H
 #define LLVM_CLANG_LEX_TOKEN_H
 
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/TokenKinds.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include <cassert>
 
 namespace clang {
 
 class IdentifierInfo;
 
 /// Token - This structure provides full information about a lexed token.
struct PPConditionalInfo {
   /// an \#else block or something.  Only useful in Skipping blocks.
   bool FoundNonSkip;
 
   /// True if we've seen a \#else in this block.  If so,
   /// \#elif/\#else directives are not allowed.
   bool FoundElse;
 };
 
+// Extra information needed for annonation tokens.
+struct PragmaLoopHintInfo {
+  Token PragmaName;
+  Token Option;
+  ArrayRef<Token> Toks;
+};
 } // end namespace clang
 
 #endif // LLVM_CLANG_LEX_TOKEN_H

clang/include/clang/Serialization/ASTBitCodes.h

 /// Whenever the AST file format changes in a way that makes it
 /// incompatible with previous versions (such that a reader
 /// designed for the previous version could not support reading
 /// the new version), this number should be increased.
 ///
 /// Version 4 of AST files also requires that the version control branch and
 /// revision match exactly, since there is no backward compatibility of
 /// AST files at this time.
-const unsigned VERSION_MAJOR = 23;
+const unsigned VERSION_MAJOR = 24;
 
 /// AST file minor version number supported by this version of
 /// Clang.
 ///
 /// Whenever the AST format changes in a way that is still
 /// compatible with previous versions (such that a reader designed
 /// for the previous version could still support reading the new
 /// version by ignoring new kinds of subblocks), this number

clang/lib/Parse/ParsePragma.cpp

if (ExpectAndConsume(tok::r_paren, diag::warn_pragma_expected_rparen,
       ExpectAndConsume(tok::eof, diag::warn_pragma_extra_tokens_at_eol,
                        PragmaName))
     return false;
 
   Actions.ActOnPragmaMSAllocText(FirstTok.getLocation(), Section, Functions);
   return true;
 }
 
-namespace {
-struct PragmaLoopHintInfo {
-  Token PragmaName;
-  Token Option;
-  ArrayRef<Token> Toks;
-};
-} // end anonymous namespace
-
 static std::string PragmaLoopHintString(Token PragmaName, Token Option) {
   StringRef Str = PragmaName.getIdentifierInfo()->getName();
   std::string ClangLoopStr("clang loop ");
   if (Str == "loop" && Option.getIdentifierInfo())
     ClangLoopStr += Option.getIdentifierInfo()->getName();
   return std::string(llvm::StringSwitch<StringRef>(Str)
                          .Case("loop", ClangLoopStr)
                          .Case("unroll_and_jam", Str)

clang/lib/Serialization/ASTReader.cpp

llvm::Error ASTReader::ReadBlockAbbrevs(BitstreamCursor &Cursor,
   }
 }
 
 Token ASTReader::ReadToken(ModuleFile &F, const RecordDataImpl &Record,
                            unsigned &Idx) {
   Token Tok;
   Tok.startToken();
   Tok.setLocation(ReadSourceLocation(F, Record, Idx));
+  Tok.setKind((tok::TokenKind)Record[Idx++]);
+  Tok.setFlag((Token::TokenFlags)Record[Idx++]);
+
+  if (Tok.isAnnotation()) {
+    Tok.setAnnotationEndLoc(ReadSourceLocation(F, Record, Idx));
+    switch (Tok.getKind()) {
+    case tok::annot_pragma_loop_hint: {
+      auto *Info = new (PP.getPreprocessorAllocator()) PragmaLoopHintInfo;
+      Info->PragmaName = ReadToken(F, Record, Idx);
+      Info->Option = ReadToken(F, Record, Idx);
+      unsigned NumTokens = Record[Idx++];
+      SmallVector<Token, 4> Toks;
+      Toks.reserve(NumTokens);
+      for (unsigned I = 0; I < NumTokens; ++I)
+        Toks.push_back(ReadToken(F, Record, Idx));
+      Info->Toks = llvm::makeArrayRef(Toks).copy(PP.getPreprocessorAllocator());
+      Tok.setAnnotationValue(static_cast<void *>(Info));
+      break;
+    }
+    // Some annotation tokens do not use the PtrData field.
+    case tok::annot_pragma_openmp:
+    case tok::annot_pragma_openmp_end:
+    case tok::annot_pragma_unused:
+      break;
+    default:
+      llvm_unreachable("missing deserialization code for annotation token");
+    }
+  } else {
     Tok.setLength(Record[Idx++]);
     if (IdentifierInfo *II = getLocalIdentifier(F, Record[Idx++]))
       Tok.setIdentifierInfo(II);
-  Tok.setKind((tok::TokenKind)Record[Idx++]);
-  Tok.setFlag((Token::TokenFlags)Record[Idx++]);
+  }
   return Tok;
 }
 
 MacroInfo *ASTReader::ReadMacroRecord(ModuleFile &F, uint64_t Offset) {
   BitstreamCursor &Stream = F.MacroCursor;
 
   // Keep track of where we are in the stream, then jump back there
   // after reading this macro.

clang/lib/Serialization/ASTWriter.cpp

 void ASTRecordWriter::AddAttributes(ArrayRef<const Attr *> Attrs) {
   push_back(Attrs.size());
   for (const auto *A : Attrs)
     AddAttr(A);
 }
 
 void ASTWriter::AddToken(const Token &Tok, RecordDataImpl &Record) {
   AddSourceLocation(Tok.getLocation(), Record);
-  Record.push_back(Tok.getLength());
-
-  // FIXME: When reading literal tokens, reconstruct the literal pointer
-  // if it is needed.
-  AddIdentifierRef(Tok.getIdentifierInfo(), Record);
   // FIXME: Should translate token kind to a stable encoding.
   Record.push_back(Tok.getKind());
   // FIXME: Should translate token flags to a stable encoding.
   Record.push_back(Tok.getFlags());
+
+  if (Tok.isAnnotation()) {
+    AddSourceLocation(Tok.getAnnotationEndLoc(), Record);
+    switch (Tok.getKind()) {
+    case tok::annot_pragma_loop_hint: {
+      auto *Info = static_cast<PragmaLoopHintInfo *>(Tok.getAnnotationValue());
+      AddToken(Info->PragmaName, Record);
+      AddToken(Info->Option, Record);
+      Record.push_back(Info->Toks.size());
+      for (const auto &T : Info->Toks)
+        AddToken(T, Record);
+      break;
+    }
+    // Some annotation tokens do not use the PtrData field.
+    case tok::annot_pragma_openmp:
+    case tok::annot_pragma_openmp_end:
+    case tok::annot_pragma_unused:
+      break;
+    default:
+      llvm_unreachable("missing serialization code for annotation token");
+    }
+  } else {
+    Record.push_back(Tok.getLength());
+    // FIXME: When reading literal tokens, reconstruct the literal pointer if it
+    // is needed.
+    AddIdentifierRef(Tok.getIdentifierInfo(), Record);
+  }
 }
 
 void ASTWriter::AddString(StringRef Str, RecordDataImpl &Record) {
   Record.push_back(Str.size());
   Record.insert(Record.end(), Str.begin(), Str.end());
 }
 
 bool ASTWriter::PreparePathForOutput(SmallVectorImpl<char> &Path) {

clang/test/PCH/delayed-template-with-pragma.cpp

+// RUN: %clang_cc1 -fopenmp -emit-pch -o %t.pch %s
+// RUN: %clang_cc1 -fopenmp -fdelayed-template-parsing -emit-pch -o %t.delayed.pch %s
+// RUN: %clang_cc1 -DMAIN_FILE -fopenmp -include-pch %t.pch \
+// RUN:   -emit-llvm -o - %s -fopenmp | FileCheck %s
+// RUN: %clang_cc1 -DMAIN_FILE -fopenmp -fdelayed-template-parsing -verify \
+// RUN:   -Wunused-variable -include-pch %t.delayed.pch \
+// RUN:   -emit-llvm -o - %s | FileCheck %s
+
+#ifndef MAIN_FILE
+template <typename T>
+void a(T t) {
+  #pragma clang loop unroll_count(4)
+  for(int i=0;i<8;++i) {}
+  #pragma omp simd
+  for(int i=0;i<8;++i) {}
+  {
+    int x, y, z, zz;
+    #pragma unused(x)
+    #pragma unused(y, z)
+  }
+}
+#else
+// CHECK: !llvm.loop !3
+// CHECK: !llvm.loop !7
+// CHECK: !3 = distinct !{!3, !4, !5}
+// CHECK: !4 = !{!"llvm.loop.mustprogress"}
+// CHECK: !5 = !{!"llvm.loop.unroll.count", i32 4}
+// CHECK: !7 = distinct !{!7, !8, !9}
+// CHECK: !8 = !{!"llvm.loop.parallel_accesses", !6}
+// CHECK: !9 = !{!"llvm.loop.vectorize.enable", i1 true}
+// expected-warning@17 {{unused variable 'zz'}}
+void foo()
+{
+  a(1);
+}
+#endif