Merged main:16cf9c9af004 into amd-gfx:251b3d8041ba

Local branch amd-gfx 251b3d8 Merged main:19b664d35266 into amd-gfx:f205aef70eb5
Remote branch main 16cf9c9 [flang][openacc] Support labeled DO loop after acc combined directive (llvm#66296)

Author: SC llvm team

clang/lib/Serialization/ASTReader.cpp

@@ -2489,6 +2489,32 @@ InputFile ASTReader::getInputFile(ModuleFile &F, unsigned ID, bool Complain) {
     std::optional<int64_t> Old = std::nullopt;
     std::optional<int64_t> New = std::nullopt;
   };
+  auto HasInputContentChanged = [&](Change OriginalChange) {
+    assert(ValidateASTInputFilesContent &&
+           "We should only check the content of the inputs with "
+           "ValidateASTInputFilesContent enabled.");
+
+    if (StoredContentHash == static_cast<uint64_t>(llvm::hash_code(-1)))
+      return OriginalChange;
+
+    auto MemBuffOrError = FileMgr.getBufferForFile(*File);
+    if (!MemBuffOrError) {
+      if (!Complain)
+        return OriginalChange;
+      std::string ErrorStr = "could not get buffer for file '";
+      ErrorStr += File->getName();
+      ErrorStr += "'";
+      Error(ErrorStr);
+      return OriginalChange;
+    }
+
+    // FIXME: hash_value is not guaranteed to be stable!
+    auto ContentHash = hash_value(MemBuffOrError.get()->getBuffer());
+    if (StoredContentHash == static_cast<uint64_t>(ContentHash))
+      return Change{Change::None};
+
+    return Change{Change::Content};
+  };
   auto HasInputFileChanged = [&]() {
     if (StoredSize != File->getSize())
       return Change{Change::Size, StoredSize, File->getSize()};
@@ -2499,33 +2525,23 @@ InputFile ASTReader::getInputFile(ModuleFile &F, unsigned ID, bool Complain) {
 
       // In case the modification time changes but not the content,
       // accept the cached file as legit.
-      if (ValidateASTInputFilesContent &&
-          StoredContentHash != static_cast<uint64_t>(llvm::hash_code(-1))) {
-        auto MemBuffOrError = FileMgr.getBufferForFile(*File);
-        if (!MemBuffOrError) {
-          if (!Complain)
-            return MTimeChange;
-          std::string ErrorStr = "could not get buffer for file '";
-          ErrorStr += File->getName();
-          ErrorStr += "'";
-          Error(ErrorStr);
-          return MTimeChange;
-        }
-
-        // FIXME: hash_value is not guaranteed to be stable!
-        auto ContentHash = hash_value(MemBuffOrError.get()->getBuffer());
-        if (StoredContentHash == static_cast<uint64_t>(ContentHash))
-          return Change{Change::None};
+      if (ValidateASTInputFilesContent)
+        return HasInputContentChanged(MTimeChange);
 
-        return Change{Change::Content};
-      }
       return MTimeChange;
     }
     return Change{Change::None};
   };
 
   bool IsOutOfDate = false;
   auto FileChange = SkipChecks ? Change{Change::None} : HasInputFileChanged();
+  // When ForceCheckCXX20ModulesInputFiles and ValidateASTInputFilesContent
+  // enabled, it is better to check the contents of the inputs. Since we can't
+  // get correct modified time information for inputs from overriden inputs.
+  if (HSOpts.ForceCheckCXX20ModulesInputFiles && ValidateASTInputFilesContent &&
+      F.StandardCXXModule && FileChange.Kind == Change::None)
+    FileChange = HasInputContentChanged(FileChange);
+
   // For an overridden file, there is nothing to validate.
   if (!Overridden && FileChange.Kind != Change::None) {
     if (Complain && !Diags.isDiagnosticInFlight()) {

clang/unittests/Serialization/CMakeLists.txt

@@ -6,6 +6,7 @@ set(LLVM_LINK_COMPONENTS
   )
 
 add_clang_unittest(SerializationTests
+  ForceCheckFileInputTest.cpp
   InMemoryModuleCacheTest.cpp
   ModuleCacheTest.cpp
   NoCommentsTest.cpp

clang/unittests/Serialization/ForceCheckFileInputTest.cpp

@@ -0,0 +1,144 @@
+//===- unittests/Serialization/ForceCheckFileInputTest.cpp - CI tests -----===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/Utils.h"
+#include "clang/Lex/HeaderSearch.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Serialization/ASTReader.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace clang;
+
+namespace {
+
+class ForceCheckFileInputTest : public ::testing::Test {
+  void SetUp() override {
+    EXPECT_FALSE(sys::fs::createUniqueDirectory("modules-test", TestDir));
+  }
+
+  void TearDown() override { sys::fs::remove_directories(TestDir); }
+
+public:
+  SmallString<256> TestDir;
+
+  void addFile(StringRef Path, StringRef Contents) {
+    EXPECT_FALSE(sys::path::is_absolute(Path));
+
+    SmallString<256> AbsPath(TestDir);
+    sys::path::append(AbsPath, Path);
+
+    EXPECT_FALSE(
+        sys::fs::create_directories(llvm::sys::path::parent_path(AbsPath)));
+
+    std::error_code EC;
+    llvm::raw_fd_ostream OS(AbsPath, EC);
+    EXPECT_FALSE(EC);
+    OS << Contents;
+  }
+};
+
+TEST_F(ForceCheckFileInputTest, ForceCheck) {
+  addFile("a.cppm", R"cpp(
+export module a;
+export int aa = 43;
+  )cpp");
+
+  std::string BMIPath = llvm::Twine(TestDir + "/a.pcm").str();
+
+  {
+    IntrusiveRefCntPtr<DiagnosticsEngine> Diags =
+        CompilerInstance::createDiagnostics(new DiagnosticOptions());
+    CreateInvocationOptions CIOpts;
+    CIOpts.Diags = Diags;
+    CIOpts.VFS = llvm::vfs::createPhysicalFileSystem();
+
+    const char *Args[] = {
+        "clang++",       "-std=c++20", "--precompile", "-working-directory",
+        TestDir.c_str(), "a.cppm",     "-o",           BMIPath.c_str()};
+    std::shared_ptr<CompilerInvocation> Invocation =
+        createInvocation(Args, CIOpts);
+    EXPECT_TRUE(Invocation);
+
+    auto Buf = CIOpts.VFS->getBufferForFile("a.cppm");
+    EXPECT_TRUE(Buf);
+
+    Invocation->getPreprocessorOpts().addRemappedFile("a.cppm", Buf->get());
+
+    Buf->release();
+
+    CompilerInstance Instance;
+    Instance.setDiagnostics(Diags.get());
+    Instance.setInvocation(Invocation);
+
+    if (auto VFSWithRemapping = createVFSFromCompilerInvocation(
+            Instance.getInvocation(), Instance.getDiagnostics(), CIOpts.VFS))
+      CIOpts.VFS = VFSWithRemapping;
+    Instance.createFileManager(CIOpts.VFS);
+
+    Instance.getHeaderSearchOpts().ValidateASTInputFilesContent = true;
+
+    GenerateModuleInterfaceAction Action;
+    EXPECT_TRUE(Instance.ExecuteAction(Action));
+    EXPECT_FALSE(Diags->hasErrorOccurred());
+  }
+
+  {
+    IntrusiveRefCntPtr<DiagnosticsEngine> Diags =
+        CompilerInstance::createDiagnostics(new DiagnosticOptions());
+    CreateInvocationOptions CIOpts;
+    CIOpts.Diags = Diags;
+    CIOpts.VFS = llvm::vfs::createPhysicalFileSystem();
+
+    std::string BMIPath = llvm::Twine(TestDir + "/a.pcm").str();
+    const char *Args[] = {
+        "clang++",       "-std=c++20", "--precompile", "-working-directory",
+        TestDir.c_str(), "a.cppm",     "-o",           BMIPath.c_str()};
+    std::shared_ptr<CompilerInvocation> Invocation =
+        createInvocation(Args, CIOpts);
+    EXPECT_TRUE(Invocation);
+
+    CompilerInstance Clang;
+
+    Clang.setInvocation(Invocation);
+    Clang.setDiagnostics(Diags.get());
+    FileManager *FM = Clang.createFileManager(CIOpts.VFS);
+    Clang.createSourceManager(*FM);
+
+    EXPECT_TRUE(Clang.createTarget());
+    Clang.createPreprocessor(TU_Complete);
+    Clang.getHeaderSearchOpts().ForceCheckCXX20ModulesInputFiles = true;
+    Clang.getHeaderSearchOpts().ValidateASTInputFilesContent = true;
+    Clang.createASTReader();
+
+    addFile("a.cppm", R"cpp(
+export module a;
+export int aa = 44;
+  )cpp");
+
+    auto ReadResult =
+        Clang.getASTReader()->ReadAST(BMIPath, serialization::MK_MainFile,
+                                      SourceLocation(), ASTReader::ARR_None);
+
+    // We shall be able to detect the content change here.
+    EXPECT_NE(ReadResult, ASTReader::Success);
+  }
+}
+
+} // anonymous namespace

flang/include/flang/Parser/parse-tree.h

@@ -4247,7 +4247,11 @@ struct OpenACCDeclarativeConstruct {
 EMPTY_CLASS(AccEndLoop);
 struct OpenACCLoopConstruct {
   TUPLE_CLASS_BOILERPLATE(OpenACCLoopConstruct);
-  std::tuple<AccBeginLoopDirective, DoConstruct, std::optional<AccEndLoop>> t;
+  OpenACCLoopConstruct(AccBeginLoopDirective &&a)
+      : t({std::move(a), std::nullopt, std::nullopt}) {}
+  std::tuple<AccBeginLoopDirective, std::optional<DoConstruct>,
+      std::optional<AccEndLoop>>
+      t;
 };
 
 struct OpenACCStandaloneConstruct {

flang/lib/Lower/DirectivesCommon.h

@@ -587,7 +587,31 @@ void genOmpAccAtomicCapture(Fortran::lower::AbstractConverter &converter,
   firOpBuilder.setInsertionPointToStart(&block);
 }
 
+/// Create empty blocks for the current region.
+/// These blocks replace blocks parented to an enclosing region.
+template <typename... TerminatorOps>
+void createEmptyRegionBlocks(
+    fir::FirOpBuilder &builder,
+    std::list<Fortran::lower::pft::Evaluation> &evaluationList) {
+  mlir::Region *region = &builder.getRegion();
+  for (Fortran::lower::pft::Evaluation &eval : evaluationList) {
+    if (eval.block) {
+      if (eval.block->empty()) {
+        eval.block->erase();
+        eval.block = builder.createBlock(region);
+      } else {
+        [[maybe_unused]] mlir::Operation &terminatorOp = eval.block->back();
+        assert(mlir::isa<TerminatorOps...>(terminatorOp) &&
+               "expected terminator op");
+      }
+    }
+    if (!eval.isDirective() && eval.hasNestedEvaluations())
+      createEmptyRegionBlocks<TerminatorOps...>(builder,
+                                                eval.getNestedEvaluations());
+  }
+}
+
 } // namespace lower
 } // namespace Fortran
 
-#endif // FORTRAN_LOWER_DIRECTIVES_COMMON_H
+#endif // FORTRAN_LOWER_DIRECTIVES_COMMON_H