https://github.com/pgerell updated https://github.com/llvm/llvm-project/pull/220058
>From f06914c3365b780a4e97bf75278e2c413a687d97 Mon Sep 17 00:00:00 2001 From: Peter Gerell <[email protected]> Date: Mon, 31 Aug 2026 17:49:29 +0200 Subject: [PATCH] [clang][OpenMP] Keep 'requires' directives read from an AST file --- clang/docs/ReleaseNotes.md | 2 ++ clang/include/clang/Sema/SemaOpenMP.h | 6 ++++ .../include/clang/Serialization/ASTBitCodes.h | 3 ++ clang/include/clang/Serialization/ASTReader.h | 3 ++ clang/include/clang/Serialization/ASTWriter.h | 1 + clang/lib/Sema/SemaOpenMP.cpp | 12 ++++++++ clang/lib/Serialization/ASTReader.cpp | 13 +++++++++ clang/lib/Serialization/ASTWriter.cpp | 16 +++++++++++ clang/test/OpenMP/requires_module.cpp | 28 +++++++++++++++++++ clang/test/OpenMP/requires_pch.cpp | 24 ++++++++++++++++ 10 files changed, 108 insertions(+) create mode 100644 clang/test/OpenMP/requires_module.cpp create mode 100644 clang/test/OpenMP/requires_pch.cpp diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index cd59d6e942a35..e75a0762bc4b8 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -839,6 +839,8 @@ The `alpha.cplusplus.UseAfterLifetimeEnd` checker was renamed to `alpha.core.Use ### OpenMP Support +- Fixed an OpenMP `requires` directive read from a PCH or module losing its effect on + semantic checks, which caused spurious `reverse_offload` errors. - Canonicalize intra-tiles in loop tiling. `#pragma omp tile` still emits a min-bounded inner loop, which vectorizes well. When a parent directive such as `for collapse(n)` needs a constant per-tile trip count, Clang rereads a diff --git a/clang/include/clang/Sema/SemaOpenMP.h b/clang/include/clang/Sema/SemaOpenMP.h index a5f357c15f5c4..c799254632829 100644 --- a/clang/include/clang/Sema/SemaOpenMP.h +++ b/clang/include/clang/Sema/SemaOpenMP.h @@ -259,6 +259,12 @@ class SemaOpenMP : public SemaBase { /// Called on well-formed '#pragma omp requires'. DeclGroupPtrTy ActOnOpenMPRequiresDirective(SourceLocation Loc, ArrayRef<OMPClause *> ClauseList); + + /// Registers a 'requires' directive deserialized from an AST file. + void addRequiresDecl(OMPRequiresDecl *D); + + /// The 'requires' directives seen so far in this translation unit. + ArrayRef<const OMPRequiresDecl *> getRequiresDecls() const; /// Check restrictions on Requires directive OMPRequiresDecl *CheckOMPRequiresDecl(SourceLocation Loc, ArrayRef<OMPClause *> Clauses); diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h index 6a52a9e4fa780..08076b4a209ce 100644 --- a/clang/include/clang/Serialization/ASTBitCodes.h +++ b/clang/include/clang/Serialization/ASTBitCodes.h @@ -747,6 +747,9 @@ enum ASTRecordTypes { /// Record that encodes the number of submodules, their base ID in the AST /// file, and for each module the relative bit offset into the stream. SUBMODULE_METADATA = 80, + + /// Record code for the OpenMP 'requires' directives seen in the TU. + OMP_REQUIRES_DECLS = 81, }; /// Record types used within a source manager block. diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index 0c8c92feee176..c06d70a340ad3 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -1052,6 +1052,9 @@ class ASTReader : public ExternalPreprocessorSource, /// The IDs of all decls with function effects to be checked. SmallVector<GlobalDeclID> DeclsWithEffectsToVerify; + /// OpenMP 'requires' directives read from the AST file. + SmallVector<GlobalDeclID> OpenMPRequiresDecls; + /// The RISC-V intrinsic pragma(including RVV, SiFive and Andes). SmallVector<bool, 3> RISCVVecIntrinsicPragma; diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h index 95ae8a6ba8c74..f69646d1ca0a9 100644 --- a/clang/include/clang/Serialization/ASTWriter.h +++ b/clang/include/clang/Serialization/ASTWriter.h @@ -648,6 +648,7 @@ class ASTWriter : public ASTDeserializationListener, void WritePackPragmaOptions(Sema &SemaRef); void WriteFloatControlPragmaOptions(Sema &SemaRef); void WriteDeclsWithEffectsToVerify(Sema &SemaRef); + void WriteOpenMPRequiresDecls(Sema &SemaRef); void WriteModuleFileExtension(Sema &SemaRef, ModuleFileExtensionWriter &Writer); void WriteRISCVIntrinsicPragmas(Sema &SemaRef); diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index 2e4d9f2f82f0b..cf49ad6cd6e63 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -680,6 +680,10 @@ class DSAStackTy { /// Add requires decl to internal vector void addRequiresDecl(OMPRequiresDecl *RD) { RequiresDecls.push_back(RD); } + ArrayRef<const OMPRequiresDecl *> getRequiresDecls() const { + return RequiresDecls; + } + /// Checks if the defined 'requires' directive has specified type of clause. template <typename ClauseType> bool hasRequiresDeclWithClause() const { return llvm::any_of(RequiresDecls, [](const OMPRequiresDecl *D) { @@ -2071,6 +2075,14 @@ void SemaOpenMP::InitDataSharingAttributesStack() { #define DSAStack static_cast<DSAStackTy *>(VarDataSharingAttributesStack) +void SemaOpenMP::addRequiresDecl(OMPRequiresDecl *D) { + DSAStack->addRequiresDecl(D); +} + +ArrayRef<const OMPRequiresDecl *> SemaOpenMP::getRequiresDecls() const { + return DSAStack->getRequiresDecls(); +} + void SemaOpenMP::pushOpenMPFunctionRegion() { DSAStack->pushFunction(); } void SemaOpenMP::popOpenMPFunctionRegion(const FunctionScopeInfo *OldFSI) { diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index a9c230d767c50..06dd25f5db365 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -26,6 +26,7 @@ #include "clang/AST/DeclFriend.h" #include "clang/AST/DeclGroup.h" #include "clang/AST/DeclObjC.h" +#include "clang/AST/DeclOpenMP.h" #include "clang/AST/DeclTemplate.h" #include "clang/AST/DeclarationName.h" #include "clang/AST/Expr.h" @@ -81,6 +82,7 @@ #include "clang/Sema/Sema.h" #include "clang/Sema/SemaCUDA.h" #include "clang/Sema/SemaObjC.h" +#include "clang/Sema/SemaOpenMP.h" #include "clang/Sema/SemaRISCV.h" #include "clang/Sema/Weak.h" #include "clang/Serialization/ASTBitCodes.h" @@ -4480,6 +4482,11 @@ llvm::Error ASTReader::ReadASTBlock(ModuleFile &F, DeclsWithEffectsToVerify.push_back(ReadDeclID(F, Record, I)); break; + case OMP_REQUIRES_DECLS: + for (unsigned I = 0, N = Record.size(); I != N; /*in loop*/) + OpenMPRequiresDecls.push_back(ReadDeclID(F, Record, I)); + break; + case OPENCL_EXTENSIONS: for (unsigned I = 0, E = Record.size(); I != E; ) { auto Name = ReadString(Record, I); @@ -9293,6 +9300,12 @@ void ASTReader::InitializeSema(Sema &S) { void ASTReader::UpdateSema() { assert(SemaObj && "no Sema to update"); + // UpdateSema() runs after each AST file is loaded, not only the first, so a + // 'requires' directive from a module is registered too. + for (GlobalDeclID ID : OpenMPRequiresDecls) + SemaObj->OpenMP().addRequiresDecl(cast<OMPRequiresDecl>(GetDecl(ID))); + OpenMPRequiresDecls.clear(); + // Load the offsets of the declarations that Sema references. // They will be lazily deserialized when needed. if (!SemaDeclRefs.empty()) { diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index de985b770cb01..522ecc876e3d3 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -24,6 +24,7 @@ #include "clang/AST/DeclContextInternals.h" #include "clang/AST/DeclFriend.h" #include "clang/AST/DeclObjC.h" +#include "clang/AST/DeclOpenMP.h" #include "clang/AST/DeclTemplate.h" #include "clang/AST/DeclarationName.h" #include "clang/AST/Expr.h" @@ -70,6 +71,7 @@ #include "clang/Sema/Sema.h" #include "clang/Sema/SemaCUDA.h" #include "clang/Sema/SemaObjC.h" +#include "clang/Sema/SemaOpenMP.h" #include "clang/Sema/SemaRISCV.h" #include "clang/Sema/Weak.h" #include "clang/Serialization/ASTBitCodes.h" @@ -5274,6 +5276,19 @@ void ASTWriter::WriteDeclsWithEffectsToVerify(Sema &SemaRef) { Stream.EmitRecord(DECLS_WITH_EFFECTS_TO_VERIFY, Record); } +/// Write the OpenMP 'requires' directives seen in this translation unit. +void ASTWriter::WriteOpenMPRequiresDecls(Sema &SemaRef) { + if (!SemaRef.getLangOpts().OpenMP) + return; + ArrayRef<const OMPRequiresDecl *> Decls = SemaRef.OpenMP().getRequiresDecls(); + if (Decls.empty()) + return; + RecordData Record; + for (const auto *D : Decls) + AddDeclRef(D, Record); + Stream.EmitRecord(OMP_REQUIRES_DECLS, Record); +} + void ASTWriter::WriteModuleFileExtension(Sema &SemaRef, ModuleFileExtensionWriter &Writer) { // Enter the extension block. @@ -6349,6 +6364,7 @@ ASTFileSignature ASTWriter::WriteASTCore(Sema *SemaPtr, StringRef isysroot, WritePackPragmaOptions(*SemaPtr); WriteFloatControlPragmaOptions(*SemaPtr); WriteDeclsWithEffectsToVerify(*SemaPtr); + WriteOpenMPRequiresDecls(*SemaPtr); } // Some simple statistics diff --git a/clang/test/OpenMP/requires_module.cpp b/clang/test/OpenMP/requires_module.cpp new file mode 100644 index 0000000000000..7e7034473b224 --- /dev/null +++ b/clang/test/OpenMP/requires_module.cpp @@ -0,0 +1,28 @@ +// RUN: rm -rf %t && split-file %s %t +// RUN: %clang_cc1 -fopenmp -fopenmp-version=51 -fmodules -fmodule-name=rev \ +// RUN: -x c++ -emit-module %t/module.modulemap -o %t/rev.pcm +// RUN: %clang_cc1 -fopenmp -fopenmp-version=51 -fmodules -fmodule-file=%t/rev.pcm \ +// RUN: -verify -fsyntax-only %t/use.cpp +// RUN: %clang_cc1 -fopenmp -fopenmp-version=51 -fopenmp-targets=x86_64 -triple x86_64 \ +// RUN: -fmodules -fmodule-name=rev -x c++ -emit-module %t/module.modulemap -o %t/rev2.pcm +// RUN: %clang_cc1 -fopenmp -fopenmp-version=51 -fopenmp-targets=x86_64 -triple x86_64 \ +// RUN: -fmodules -fmodule-file=%t/rev2.pcm -verify -fsyntax-only %t/use.cpp + +// A 'requires' directive read from a module must keep its effect on the +// translation unit importing it. + +//--- module.modulemap +module rev { header "rev.h" export * } + +//--- rev.h +#pragma omp requires reverse_offload +void foo(); + +//--- use.cpp +#include "rev.h" + +// expected-no-diagnostics +void bar(int argc) { +#pragma omp target device(ancestor : argc) + foo(); +} diff --git a/clang/test/OpenMP/requires_pch.cpp b/clang/test/OpenMP/requires_pch.cpp new file mode 100644 index 0000000000000..7f1e76252ac6c --- /dev/null +++ b/clang/test/OpenMP/requires_pch.cpp @@ -0,0 +1,24 @@ +// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=51 -x c++ -std=c++11 -emit-pch -o %t %s +// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=51 -std=c++11 -include-pch %t -fsyntax-only %s +// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=51 -x c++ -std=c++11 -emit-pch -o %t %s +// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=51 -std=c++11 -include-pch %t -fsyntax-only %s +// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=51 -fopenmp-targets=x86_64 \ +// RUN: -triple x86_64 -x c++ -std=c++11 -emit-pch -o %t %s +// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=51 -fopenmp-targets=x86_64 \ +// RUN: -triple x86_64 -std=c++11 -include-pch %t -fsyntax-only %s + +// expected-no-diagnostics + +// A 'requires' directive read from an AST file must keep its effect on the +// translation unit including it. + +#ifndef HEADER +#define HEADER +#pragma omp requires reverse_offload +void foo(); +#else +void bar(int argc) { +#pragma omp target device(ancestor : argc) + foo(); +} +#endif _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
