Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flang][driver][nfc] Move the definition of SemanticsContext #73669

Merged
merged 1 commit into from
Nov 29, 2023

Conversation

banach-space
Copy link
Contributor

Moves the defintion of SemanticsContext within the Flang driver.

Rather than in CompilerInvocation, semantic context fits better within
CompilerInstance that encapsulates the objects that are required to run the
frontend. CompilerInvocation is better suited for objects
encapsulating compiler configuration (e.g. set-up resulting from user
input or host set-up).

@llvmbot llvmbot added flang:driver flang Flang issues not falling into any other category labels Nov 28, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 28, 2023

@llvm/pr-subscribers-flang-driver

Author: Andrzej Warzyński (banach-space)

Changes

Moves the defintion of SemanticsContext within the Flang driver.

Rather than in CompilerInvocation, semantic context fits better within
CompilerInstance that encapsulates the objects that are required to run the
frontend. CompilerInvocation is better suited for objects
encapsulating compiler configuration (e.g. set-up resulting from user
input or host set-up).


Full diff: https://github.com/llvm/llvm-project/pull/73669.diff

6 Files Affected:

  • (modified) flang/include/flang/Frontend/CompilerInstance.h (+8)
  • (modified) flang/include/flang/Frontend/CompilerInvocation.h (+2-6)
  • (modified) flang/lib/Frontend/CompilerInstance.cpp (+2-2)
  • (modified) flang/lib/Frontend/CompilerInvocation.cpp (+5-2)
  • (modified) flang/lib/Frontend/FrontendAction.cpp (+2-3)
  • (modified) flang/lib/Frontend/FrontendActions.cpp (+9-11)
diff --git a/flang/include/flang/Frontend/CompilerInstance.h b/flang/include/flang/Frontend/CompilerInstance.h
index ed71f038c4dd85e..8d504a87afa8de0 100644
--- a/flang/include/flang/Frontend/CompilerInstance.h
+++ b/flang/include/flang/Frontend/CompilerInstance.h
@@ -55,6 +55,8 @@ class CompilerInstance {
 
   std::unique_ptr<Fortran::semantics::RuntimeDerivedTypeTables> rtTyTables;
 
+  std::unique_ptr<Fortran::semantics::SemanticsContext> semaContext;
+
   /// The stream for diagnostics from Semantics
   llvm::raw_ostream *semaOutputStream = &llvm::errs();
 
@@ -81,6 +83,12 @@ class CompilerInstance {
   std::unique_ptr<llvm::raw_pwrite_stream> outputStream;
 
 public:
+  Fortran::semantics::SemanticsContext &getSemanticsContext() {
+    return *semaContext;
+  }
+  const Fortran::semantics::SemanticsContext &getSemanticsContext() const {
+    return *semaContext;
+  }
   explicit CompilerInstance();
 
   ~CompilerInstance();
diff --git a/flang/include/flang/Frontend/CompilerInvocation.h b/flang/include/flang/Frontend/CompilerInvocation.h
index 229aa75748f725d..492817a3733fb11 100644
--- a/flang/include/flang/Frontend/CompilerInvocation.h
+++ b/flang/include/flang/Frontend/CompilerInvocation.h
@@ -159,12 +159,8 @@ class CompilerInvocation : public CompilerInvocationBase {
     return loweringOpts;
   }
 
-  Fortran::semantics::SemanticsContext &getSemanticsContext() {
-    return *semanticsContext;
-  }
-  const Fortran::semantics::SemanticsContext &getSemanticsContext() const {
-    return *semanticsContext;
-  }
+  std::unique_ptr<Fortran::semantics::SemanticsContext>
+  getSemanticsCtx(Fortran::parser::AllCookedSources &allCookedSources);
 
   std::string &getModuleDir() { return moduleDir; }
   const std::string &getModuleDir() const { return moduleDir; }
diff --git a/flang/lib/Frontend/CompilerInstance.cpp b/flang/lib/Frontend/CompilerInstance.cpp
index 21ebf52f76410bd..328b3774bc29175 100644
--- a/flang/lib/Frontend/CompilerInstance.cpp
+++ b/flang/lib/Frontend/CompilerInstance.cpp
@@ -156,8 +156,8 @@ bool CompilerInstance::executeAction(FrontendAction &act) {
   invoc.setFortranOpts();
   // Set the encoding to read all input files in based on user input.
   allSources->set_encoding(invoc.getFortranOpts().encoding);
-  // Create the semantics context and set semantic options.
-  invoc.setSemanticsOpts(*this->allCookedSources);
+  // Create the semantics context
+  semaContext = invoc.getSemanticsCtx(*allCookedSources);
   // Set options controlling lowering to FIR.
   invoc.setLoweringOptions();
 
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index 1c09ae9c281eb47..654eee6a6e62587 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -1335,11 +1335,12 @@ void CompilerInvocation::setFortranOpts() {
     fortranOptions.features.WarnOnAllUsage();
 }
 
-void CompilerInvocation::setSemanticsOpts(
+std::unique_ptr<Fortran::semantics::SemanticsContext>
+CompilerInvocation::getSemanticsCtx(
     Fortran::parser::AllCookedSources &allCookedSources) {
   auto &fortranOptions = getFortranOpts();
 
-  semanticsContext = std::make_unique<semantics::SemanticsContext>(
+  auto semanticsContext = std::make_unique<semantics::SemanticsContext>(
       getDefaultKinds(), fortranOptions.features, allCookedSources);
 
   semanticsContext->set_moduleDirectory(getModuleDir())
@@ -1363,6 +1364,8 @@ void CompilerInvocation::setSemanticsOpts(
 
   if (targetTriple.isPPC())
     semanticsContext->targetCharacteristics().set_isPPC(true);
+
+  return semanticsContext;
 }
 
 /// Set \p loweringOptions controlling lowering behavior based
diff --git a/flang/lib/Frontend/FrontendAction.cpp b/flang/lib/Frontend/FrontendAction.cpp
index 02052fc5ae41c46..599b4e11f0cfbd1 100644
--- a/flang/lib/Frontend/FrontendAction.cpp
+++ b/flang/lib/Frontend/FrontendAction.cpp
@@ -171,7 +171,7 @@ bool FrontendAction::runSemanticChecks() {
 
   // Prepare semantics
   ci.setSemantics(std::make_unique<Fortran::semantics::Semantics>(
-      ci.getInvocation().getSemanticsContext(), *parseTree,
+      ci.getSemanticsContext(), *parseTree,
       ci.getInvocation().getDebugModuleDir()));
   auto &semantics = ci.getSemantics();
 
@@ -191,8 +191,7 @@ bool FrontendAction::runSemanticChecks() {
 bool FrontendAction::generateRtTypeTables() {
   getInstance().setRtTyTables(
       std::make_unique<Fortran::semantics::RuntimeDerivedTypeTables>(
-          BuildRuntimeDerivedTypeTables(
-              getInstance().getInvocation().getSemanticsContext())));
+          BuildRuntimeDerivedTypeTables(getInstance().getSemanticsContext())));
 
   // The runtime derived type information table builder may find additional
   // semantic errors. Report them.
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index f573ac82c91cd8e..9a35b396ed383b6 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -330,13 +330,13 @@ bool CodeGenAction::beginSourceFileAction() {
 
   // Create a LoweringBridge
   const common::IntrinsicTypeDefaultKinds &defKinds =
-      ci.getInvocation().getSemanticsContext().defaultKinds();
+      ci.getSemanticsContext().defaultKinds();
   fir::KindMapping kindMap(mlirCtx.get(), llvm::ArrayRef<fir::KindTy>{
                                               fir::fromDefaultKinds(defKinds)});
   lower::LoweringBridge lb = Fortran::lower::LoweringBridge::create(
-      *mlirCtx, ci.getInvocation().getSemanticsContext(), defKinds,
-      ci.getInvocation().getSemanticsContext().intrinsics(),
-      ci.getInvocation().getSemanticsContext().targetCharacteristics(),
+      *mlirCtx, ci.getSemanticsContext(), defKinds,
+      ci.getSemanticsContext().intrinsics(),
+      ci.getSemanticsContext().targetCharacteristics(),
       ci.getParsing().allCooked(), ci.getInvocation().getTargetOpts().triple,
       kindMap, ci.getInvocation().getLoweringOpts(),
       ci.getInvocation().getFrontendOpts().envDefaults,
@@ -363,7 +363,7 @@ bool CodeGenAction::beginSourceFileAction() {
 
   // Create a parse tree and lower it to FIR
   Fortran::parser::Program &parseTree{*ci.getParsing().parseTree()};
-  lb.lower(parseTree, ci.getInvocation().getSemanticsContext());
+  lb.lower(parseTree, ci.getSemanticsContext());
 
   // Add dependent libraries
   addDepdendentLibs(*mlirModule, ci);
@@ -634,8 +634,8 @@ void DebugPreFIRTreeAction::executeAction() {
   auto &parseTree{*ci.getParsing().parseTree()};
 
   // Dump pre-FIR tree
-  if (auto ast{Fortran::lower::createPFT(
-          parseTree, ci.getInvocation().getSemanticsContext())}) {
+  if (auto ast{
+          Fortran::lower::createPFT(parseTree, ci.getSemanticsContext())}) {
     Fortran::lower::dumpPFT(llvm::outs(), *ast);
   } else {
     unsigned diagID = ci.getDiagnostics().getCustomDiagID(
@@ -673,10 +673,8 @@ void GetDefinitionAction::executeAction() {
 
   llvm::outs() << "String range: >" << charBlock->ToString() << "<\n";
 
-  auto *symbol{ci.getInvocation()
-                   .getSemanticsContext()
-                   .FindScope(*charBlock)
-                   .FindSymbol(*charBlock)};
+  auto *symbol{
+      ci.getSemanticsContext().FindScope(*charBlock).FindSymbol(*charBlock)};
   if (!symbol) {
     ci.getDiagnostics().Report(diagID);
     return;

Copy link
Contributor

@jeanPerier jeanPerier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for this change!

Moves the defintion of `SemanticsContext` within the Flang driver.

Rather than in `CompilerInvocation`, semantic context fits better within
`CompilerInstance` that encapsulates the objects that are required to run the
frontend. `CompilerInvocation` is better suited for objects
encapsulating compiler configuration (e.g. set-up resulting from user
input or host set-up).
Copy link
Contributor

@kiranchandramohan kiranchandramohan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Thanks for your help with this @banach-space.

@banach-space banach-space merged commit ae4d7ac into llvm:main Nov 29, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:driver flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants