[flang] Updating drivers to create data layout before semantics #73301
Conversation
jeanPerier
requested review from
sscalpone,
clementval,
banach-space,
tblah,
kiranchandramohan,
DanielCChen,
kkwli,
DominikAdamski and
vzakhari
llvmbot
added
flang:driver
flang
|
@llvm/pr-subscribers-flang-semantics @llvm/pr-subscribers-flang-driver Author: None (jeanPerier) ChangesPreliminary patch to change lowering/code generation to use llvm::DataLayout information instead of generating "sizeof" GEP (see #71507). Fortran Semantic analysis needs to know about the target type size and alignment to deal with common blocks, and intrinsics like C_SIZEOF/TRANSFER. This information should be obtained from the llvm::DataLayout so that it is consistent during the whole compilation flow. This change is changing flang-new and bbc drivers to:
For flang-new, the changes are code shuffling: the As a consequence, LLVM targets must be registered when running semantics, and it is not possible to run semantics for a target that is not registered with the -triple option (hence the power pc specific modules can only be built if the PowerPC target is available, @DanielCChen and @kkwli , I think I got it right, but you may want to double check the Patch is 47.80 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/73301.diff 21 Files Affected:
diff --git a/flang/include/flang/Frontend/CompilerInvocation.h b/flang/include/flang/Frontend/CompilerInvocation.h
index 229aa75748f725d..acbf644af24cde4 100644
--- a/flang/include/flang/Frontend/CompilerInvocation.h
+++ b/flang/include/flang/Frontend/CompilerInvocation.h
@@ -24,6 +24,7 @@
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/DiagnosticOptions.h"
#include "llvm/Option/ArgList.h"
+#include "llvm/Target/TargetMachine.h"
#include <memory>
namespace Fortran::frontend {
@@ -113,6 +114,8 @@ class CompilerInvocation : public CompilerInvocationBase {
bool enableConformanceChecks = false;
bool enableUsageChecks = false;
+ std::unique_ptr<llvm::TargetMachine> targetMachine;
+
/// Used in e.g. unparsing to dump the analyzed rather than the original
/// parse-tree objects.
Fortran::parser::AnalyzedObjectsAsFortran asFortran{
@@ -206,6 +209,14 @@ class CompilerInvocation : public CompilerInvocationBase {
const Fortran::common::IntrinsicTypeDefaultKinds &getDefaultKinds() const {
return defaultKinds;
}
+ const llvm::TargetMachine &getTargetMachine() const {
+ assert(targetMachine && "target machine was not set");
+ return *targetMachine;
+ }
+ llvm::TargetMachine &getTargetMachine() {
+ assert(targetMachine && "target machine was not set");
+ return *targetMachine;
+ }
/// Create a compiler invocation from a list of input options.
/// \returns true on success.
@@ -260,6 +271,12 @@ class CompilerInvocation : public CompilerInvocationBase {
/// Set \p loweringOptions controlling lowering behavior based
/// on the \p optimizationLevel.
void setLoweringOptions();
+
+ /// Sets up LLVM's TargetMachine.
+ bool setUpTargetMachine(clang::DiagnosticsEngine &diags);
+
+ /// Produces the string which represents target feature
+ std::string getTargetFeatures(clang::DiagnosticsEngine &diags);
};
} // end namespace Fortran::frontend
diff --git a/flang/include/flang/Frontend/FrontendActions.h b/flang/include/flang/Frontend/FrontendActions.h
index 8272e0729ce3fc6..e2e859f3a81bd7a 100644
--- a/flang/include/flang/Frontend/FrontendActions.h
+++ b/flang/include/flang/Frontend/FrontendActions.h
@@ -21,7 +21,6 @@
#include "mlir/IR/BuiltinOps.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/IR/Module.h"
-#include "llvm/Target/TargetMachine.h"
#include <memory>
namespace Fortran::frontend {
@@ -204,8 +203,6 @@ class CodeGenAction : public FrontendAction {
void executeAction() override;
/// Runs prescan, parsing, sema and lowers to MLIR.
bool beginSourceFileAction() override;
- /// Sets up LLVM's TargetMachine.
- bool setUpTargetMachine();
/// Runs the optimization (aka middle-end) pipeline on the LLVM module
/// associated with this action.
void runOptimizationPipeline(llvm::raw_pwrite_stream &os);
@@ -234,7 +231,6 @@ class CodeGenAction : public FrontendAction {
BackendActionTy action;
- std::unique_ptr<llvm::TargetMachine> tm;
/// }
public:
~CodeGenAction() override;
diff --git a/flang/include/flang/Lower/Bridge.h b/flang/include/flang/Lower/Bridge.h
index d363068acfdbb9d..6c0d14d65edae1e 100644
--- a/flang/include/flang/Lower/Bridge.h
+++ b/flang/include/flang/Lower/Bridge.h
@@ -22,6 +22,10 @@
#include "flang/Optimizer/Dialect/Support/KindMapping.h"
#include "mlir/IR/BuiltinOps.h"
+namespace llvm {
+class DataLayout;
+} // namespace llvm
+
namespace Fortran {
namespace common {
class IntrinsicTypeDefaultKinds;
@@ -59,10 +63,12 @@ class LoweringBridge {
llvm::StringRef triple, fir::KindMapping &kindMap,
const Fortran::lower::LoweringOptions &loweringOptions,
const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults,
- const Fortran::common::LanguageFeatureControl &languageFeatures) {
+ const Fortran::common::LanguageFeatureControl &languageFeatures,
+ const llvm::DataLayout *dataLayout = nullptr) {
return LoweringBridge(ctx, semanticsContext, defaultKinds, intrinsics,
targetCharacteristics, allCooked, triple, kindMap,
- loweringOptions, envDefaults, languageFeatures);
+ loweringOptions, envDefaults, languageFeatures,
+ dataLayout);
}
//===--------------------------------------------------------------------===//
@@ -140,7 +146,8 @@ class LoweringBridge {
fir::KindMapping &kindMap,
const Fortran::lower::LoweringOptions &loweringOptions,
const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults,
- const Fortran::common::LanguageFeatureControl &languageFeatures);
+ const Fortran::common::LanguageFeatureControl &languageFeatures,
+ const llvm::DataLayout *dataLayout);
LoweringBridge() = delete;
LoweringBridge(const LoweringBridge &) = delete;
diff --git a/flang/include/flang/Optimizer/Support/DataLayout.h b/flang/include/flang/Optimizer/Support/DataLayout.h
new file mode 100644
index 000000000000000..fe683a72927df04
--- /dev/null
+++ b/flang/include/flang/Optimizer/Support/DataLayout.h
@@ -0,0 +1,39 @@
+//===-- Optimizer/Support/DataLayout.h --------------------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+//
+// Coding style: https://mlir.llvm.org/getting_started/DeveloperGuide/
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef FORTRAN_OPTIMIZER_SUPPORT_DATALAYOUT_H
+#define FORTRAN_OPTIMIZER_SUPPORT_DATALAYOUT_H
+
+namespace mlir {
+class ModuleOp;
+}
+namespace llvm {
+class DataLayout;
+}
+
+namespace fir::support {
+/// Create an mlir::DataLayoutSpecInterface attribute from an llvm::DataLayout
+/// and set it on the provided mlir::ModuleOp.
+/// Also set the llvm.data_layout attribute with the string representation of
+/// the llvm::DataLayout on the module.
+/// These attributes are replaced if they were already set.
+void setMLIRDataLayout(mlir::ModuleOp mlirModule, const llvm::DataLayout &dl);
+
+/// Create an mlir::DataLayoutSpecInterface from the llvm>data_layout attribute
+/// if one is provided. If such attribute is not available, create a default
+/// target independent layout when allowDefaultLayout is true. Otherwise do
+/// nothing.
+void setMLIRDataLayoutFromAttributes(mlir::ModuleOp mlirModule,
+ bool allowDefaultLayout);
+} // namespace fir::support
+
+#endif // FORTRAN_OPTIMIZER_SUPPORT_DATALAYOUT_H
diff --git a/flang/include/flang/Tools/TargetSetup.h b/flang/include/flang/Tools/TargetSetup.h
new file mode 100644
index 000000000000000..0b09acefde98a3c
--- /dev/null
+++ b/flang/include/flang/Tools/TargetSetup.h
@@ -0,0 +1,63 @@
+//===-- Tools/TargetSetup.h ------------------------------------- *-C++-*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef FORTRAN_TOOLS_TARGET_SETUP_H
+#define FORTRAN_TOOLS_TARGET_SETUP_H
+
+#include "flang/Evaluate/target.h"
+#include "llvm/MC/TargetRegistry.h"
+#include "llvm/Target/TargetMachine.h"
+#include "llvm/TargetParser/Host.h"
+#include <memory>
+
+namespace Fortran::tools {
+
+[[maybe_unused]] inline static void setUpTargetCharacteristics(
+ Fortran::evaluate::TargetCharacteristics &targetCharacteristics,
+ const llvm::TargetMachine &targetMachine,
+ const std::string &compilerVersion, const std::string &compilerOptions) {
+
+ const llvm::Triple &targetTriple{targetMachine.getTargetTriple()};
+ // FIXME: Handle real(3) ?
+ if (targetTriple.getArch() != llvm::Triple::ArchType::x86_64)
+ targetCharacteristics.DisableType(
+ Fortran::common::TypeCategory::Real, /*kind=*/10);
+
+ targetCharacteristics.set_compilerOptionsString(compilerOptions)
+ .set_compilerVersionString(compilerVersion);
+
+ if (targetTriple.isPPC())
+ targetCharacteristics.set_isPPC(true);
+
+ // TODO: use target machine data layout to set-up the target characteristics
+ // type size and alignment info.
+}
+
+/// Create a target machine that is at least sufficient to get data-layout
+/// information required by flang semantics and lowering. Note that it may not
+/// contain all the CPU feature information to get optimized assembly generation
+/// from LLVM IR. Drivers that needs to generate assembly from LLVM IR should
+/// create a target machine according to their specific options.
+[[maybe_unused]] inline static std::unique_ptr<llvm::TargetMachine>
+createTargetMachine(llvm::StringRef targetTriple, std::string &error) {
+ std::string triple{targetTriple};
+ if (triple.empty())
+ triple = llvm::sys::getDefaultTargetTriple();
+
+ const llvm::Target *theTarget =
+ llvm::TargetRegistry::lookupTarget(triple, error);
+ if (!theTarget)
+ return nullptr;
+ return std::unique_ptr<llvm::TargetMachine>{
+ theTarget->createTargetMachine(triple, /*CPU=*/"",
+ /*Features=*/"", llvm::TargetOptions(),
+ /*Reloc::Model=*/std::nullopt)};
+}
+} // namespace Fortran::tools
+
+#endif // FORTRAN_TOOLS_TARGET_SETUP_H
diff --git a/flang/lib/Frontend/CompilerInstance.cpp b/flang/lib/Frontend/CompilerInstance.cpp
index 21ebf52f76410bd..1e3f5034a438a24 100644
--- a/flang/lib/Frontend/CompilerInstance.cpp
+++ b/flang/lib/Frontend/CompilerInstance.cpp
@@ -156,6 +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);
+ if (!invoc.setUpTargetMachine(getDiagnostics()))
+ return false;
// Create the semantics context and set semantic options.
invoc.setSemanticsOpts(*this->allCookedSources);
// Set options controlling lowering to FIR.
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index cb4f2d6a6225205..fe53f0f2116ad66 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -18,6 +18,7 @@
#include "flang/Frontend/PreprocessorOptions.h"
#include "flang/Frontend/TargetOptions.h"
#include "flang/Semantics/semantics.h"
+#include "flang/Tools/TargetSetup.h"
#include "flang/Version.inc"
#include "clang/Basic/AllDiagnostics.h"
#include "clang/Basic/DiagnosticDriver.h"
@@ -25,9 +26,11 @@
#include "clang/Driver/DriverDiagnostic.h"
#include "clang/Driver/OptionUtils.h"
#include "clang/Driver/Options.h"
+#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSwitch.h"
#include "llvm/Frontend/Debug/Options.h"
+#include "llvm/MC/TargetRegistry.h"
#include "llvm/Option/Arg.h"
#include "llvm/Option/ArgList.h"
#include "llvm/Option/OptTable.h"
@@ -37,6 +40,7 @@
#include "llvm/Support/Process.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/TargetParser/Host.h"
+#include "llvm/TargetParser/TargetParser.h"
#include "llvm/TargetParser/Triple.h"
#include <cstdlib>
#include <memory>
@@ -1347,20 +1351,10 @@ void CompilerInvocation::setSemanticsOpts(
.set_moduleFileSuffix(getModuleFileSuffix())
.set_underscoring(getCodeGenOpts().Underscoring);
- llvm::Triple targetTriple{llvm::Triple(this->targetOpts.triple)};
- // FIXME: Handle real(3) ?
- if (targetTriple.getArch() != llvm::Triple::ArchType::x86_64) {
- semanticsContext->targetCharacteristics().DisableType(
- Fortran::common::TypeCategory::Real, /*kind=*/10);
- }
-
- std::string version = Fortran::common::getFlangFullVersion();
- semanticsContext->targetCharacteristics()
- .set_compilerOptionsString(allCompilerInvocOpts)
- .set_compilerVersionString(version);
-
- if (targetTriple.isPPC())
- semanticsContext->targetCharacteristics().set_isPPC(true);
+ std::string compilerVersion = Fortran::common::getFlangFullVersion();
+ Fortran::tools::setUpTargetCharacteristics(
+ semanticsContext->targetCharacteristics(), getTargetMachine(),
+ compilerVersion, allCompilerInvocOpts);
}
/// Set \p loweringOptions controlling lowering behavior based
@@ -1387,3 +1381,127 @@ void CompilerInvocation::setLoweringOptions() {
.setAssociativeMath(langOptions.AssociativeMath)
.setReciprocalMath(langOptions.ReciprocalMath);
}
+
+// Get feature string which represents combined explicit target features
+// for AMD GPU and the target features specified by the user
+static std::string
+getExplicitAndImplicitAMDGPUTargetFeatures(clang::DiagnosticsEngine &diags,
+ const TargetOptions &targetOpts,
+ const llvm::Triple triple) {
+ llvm::StringRef cpu = targetOpts.cpu;
+ llvm::StringMap<bool> implicitFeaturesMap;
+ std::string errorMsg;
+ // Get the set of implicit target features
+ llvm::AMDGPU::fillAMDGPUFeatureMap(cpu, triple, implicitFeaturesMap);
+
+ // Add target features specified by the user
+ for (auto &userFeature : targetOpts.featuresAsWritten) {
+ std::string userKeyString = userFeature.substr(1);
+ implicitFeaturesMap[userKeyString] = (userFeature[0] == '+');
+ }
+
+ if (!llvm::AMDGPU::insertWaveSizeFeature(cpu, triple, implicitFeaturesMap,
+ errorMsg)) {
+ unsigned diagID = diags.getCustomDiagID(clang::DiagnosticsEngine::Error,
+ "Unsupported feature ID: %0");
+ diags.Report(diagID) << errorMsg.data();
+ return std::string();
+ }
+
+ llvm::SmallVector<std::string> featuresVec;
+ for (auto &implicitFeatureItem : implicitFeaturesMap) {
+ featuresVec.push_back((llvm::Twine(implicitFeatureItem.second ? "+" : "-") +
+ implicitFeatureItem.first().str())
+ .str());
+ }
+ llvm::sort(featuresVec);
+ return llvm::join(featuresVec, ",");
+}
+
+// Get feature string which represents combined explicit target features
+// for NVPTX and the target features specified by the user/
+// TODO: Have a more robust target conf like `clang/lib/Basic/Targets/NVPTX.cpp`
+static std::string
+getExplicitAndImplicitNVPTXTargetFeatures(clang::DiagnosticsEngine &diags,
+ const TargetOptions &targetOpts,
+ const llvm::Triple triple) {
+ llvm::StringRef cpu = targetOpts.cpu;
+ llvm::StringMap<bool> implicitFeaturesMap;
+ std::string errorMsg;
+ bool ptxVer = false;
+
+ // Add target features specified by the user
+ for (auto &userFeature : targetOpts.featuresAsWritten) {
+ llvm::StringRef userKeyString(llvm::StringRef(userFeature).drop_front(1));
+ implicitFeaturesMap[userKeyString.str()] = (userFeature[0] == '+');
+ // Check if the user provided a PTX version
+ if (userKeyString.startswith("ptx"))
+ ptxVer = true;
+ }
+
+ // Set the default PTX version to `ptx61` if none was provided.
+ // TODO: set the default PTX version based on the chip.
+ if (!ptxVer)
+ implicitFeaturesMap["ptx61"] = true;
+
+ // Set the compute capability.
+ implicitFeaturesMap[cpu.str()] = true;
+
+ llvm::SmallVector<std::string> featuresVec;
+ for (auto &implicitFeatureItem : implicitFeaturesMap) {
+ featuresVec.push_back((llvm::Twine(implicitFeatureItem.second ? "+" : "-") +
+ implicitFeatureItem.first().str())
+ .str());
+ }
+ llvm::sort(featuresVec);
+ return llvm::join(featuresVec, ",");
+}
+
+std::string
+CompilerInvocation::getTargetFeatures(clang::DiagnosticsEngine &diags) {
+ const TargetOptions &targetOpts = getTargetOpts();
+ const llvm::Triple triple(targetOpts.triple);
+
+ // Clang does not append all target features to the clang -cc1 invocation.
+ // Some target features are parsed implicitly by clang::TargetInfo child
+ // class. Clang::TargetInfo classes are the basic clang classes and
+ // they cannot be reused by Flang.
+ // That's why we need to extract implicit target features and add
+ // them to the target features specified by the user
+ if (triple.isAMDGPU()) {
+ return getExplicitAndImplicitAMDGPUTargetFeatures(diags, targetOpts,
+ triple);
+ } else if (triple.isNVPTX()) {
+ return getExplicitAndImplicitNVPTXTargetFeatures(diags, targetOpts, triple);
+ }
+ return llvm::join(targetOpts.featuresAsWritten.begin(),
+ targetOpts.featuresAsWritten.end(), ",");
+}
+
+bool CompilerInvocation::setUpTargetMachine(clang::DiagnosticsEngine &diags) {
+ const std::string &theTriple = getTargetOpts().triple;
+
+ // Create `Target`
+ std::string error;
+ const llvm::Target *theTarget =
+ llvm::TargetRegistry::lookupTarget(theTriple, error);
+ if (!theTarget) {
+ diags.Report(clang::diag::err_fe_unable_to_create_target) << error;
+ return false;
+ }
+
+ // Create `TargetMachine`
+ const auto &CGOpts = getCodeGenOpts();
+ std::optional<llvm::CodeGenOptLevel> OptLevelOrNone =
+ llvm::CodeGenOpt::getLevel(CGOpts.OptimizationLevel);
+ assert(OptLevelOrNone && "Invalid optimization level!");
+ llvm::CodeGenOptLevel OptLevel = *OptLevelOrNone;
+ std::string featuresStr = getTargetFeatures(diags);
+ targetMachine.reset(theTarget->createTargetMachine(
+ theTriple, /*CPU=*/targetOpts.cpu,
+ /*Features=*/featuresStr, llvm::TargetOptions(),
+ /*Reloc::Model=*/CGOpts.getRelocationModel(),
+ /*CodeModel::Model=*/std::nullopt, OptLevel));
+ assert(tm && "Failed to create TargetMachine");
+ return true;
+}
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index f573ac82c91cd8e..1cf064366f9c6e6 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -21,6 +21,7 @@
#include "flang/Lower/Support/Verifier.h"
#include "flang/Optimizer/Dialect/Support/FIRContext.h"
#include "flang/Optimizer/Dialect/Support/KindMapping.h"
+#include "flang/Optimizer/Support/DataLayout.h"
#include "flang/Optimizer/Support/InitFIR.h"
#include "flang/Optimizer/Support/Utils.h"
#include "flang/Optimizer/Transforms/Passes.h"
@@ -53,7 +54,6 @@
#include "llvm/IR/LegacyPassManager.h"
#include "llvm/IR/Verifier.h"
#include "llvm/IRReader/IRReader.h"
-#include "llvm/MC/TargetRegistry.h"
#include "llvm/Object/OffloadBinary.h"
#include "llvm/Passes/PassBuilder.h"
#include "llvm/Passes/PassPlugin.h"
@@ -65,7 +65,6 @@
#include "llvm/Support/SourceMgr.h"
#include "llvm/Support/ToolOutputFile.h"
#include "llvm/Target/TargetMachine.h"
-#include "llvm/TargetParser/TargetParser.h"
#include "llvm/Transforms/Utils/ModuleUtils.h"
#include <memory>
#include <system_error>
@@ -139,111 +138,6 @@ bool PrescanAndSemaDebugAction::beginSourceFileAction() {
(generateRtTypeTables() || true);
}
-// Get feature string which represents combined explicit target features
-// for AMD GPU and the target features specified by the user
-static std::string
-getExplicitAndImplicitAMDGPUTargetFeatures(CompilerInstance &ci,
- const TargetOptions &targetOpts,
- const llvm::Triple triple) {
- llvm::StringRef cpu = targetOpts.cpu;
- llvm::StringMap<bool> implicitFeaturesMap;
- std::string errorMsg;
- // Get the set of implicit target features
- llvm::AMDGPU::fillAMDGPUFeatureMap(cpu, triple, implicitFeaturesMap);
-
- // Add target features specified by the user
- for (auto &userFeature : targetOpts.featuresAsWritten) {
- std::string userKeyString = userFeature.substr(1);
- implicitFeaturesMap[userKeyString] = (userFeature[0] == '+');
- }
-
- if (!llvm::AMDGPU::insertWaveSizeFeature(cpu, triple, implicitFeaturesMap,
- errorMsg)) {
- unsigned diagID = ci.getDiagnostics().getCustomDiagID(
- clang::DiagnosticsEngine::Error, "Unsupported feature ID: %0");
- ci.getDiagnostics().Report(diagID) << errorMsg.data();
- return std::string();
- }
-
- llvm::SmallVect...
[truncated]
|
|
CI seems to be failing while building the compiler. |
There was a problem hiding this comment.
Fortran Semantic analysis needs to know about the target type size and alignment to deal with common blocks, and intrinsics like C_SIZEOF/TRANSFER. This information should be obtained from the llvm::DataLayout so that it is consistent during the whole compilation flow.
Is it possible to add a semantics test for this?
| @@ -0,0 +1,9 @@ | |||
| ! Test bbc set-up of the target data layout from the host. | |||
| ! RUN: bbc %s -o - | FileCheck %s | |||
There was a problem hiding this comment.
Assuming there are existing tests for the flang-new driver and new ones are not required.
There was a problem hiding this comment.
That is right, flang-new changes are NFC aside from the timing when the TargetMachine is created.
This is already tested here:
Thanks, should be fixed.
No because the functional change to fill the SemanticsContext.targetCharacteristics from the TargetMachine is not part of this change, it is left TODO here: https://github.com/llvm/llvm-project/pull/73301/files#diff-74352fcca4246990c7d9ce870cefdb335412f10f045e0175ef242cbc79564f2eR37 The current patch only ensures the TargetMachine is created before setting up the SemanticsContext.targetCharacteristics in the drivers and that the llvm::DataLayout and its MLIR equivalent are always available during lowering/codegen. |
| else() | ||
| # Do not compile PPC module if the target is not available. | ||
| continue() | ||
| endif() |
There was a problem hiding this comment.
Yes. These modules are only built for PPC platform.
|
I've quickly scanned through and have one high level comment - would it be possible to keep the logic related to This change is driven by the need to access One reason for this ask would be to make these changes less intrusive. But that's secondary. The main reason would be to stick to the original design principles behind these classes. I think that my suggestion could be implemented by extracting @jeanPerier would that still work for you? |
I want a single place where the evaluate::TargetCharacteristics is being set-up, and it will need an llvm::DataLayout, which can only be obtained from an llvm::TargetMachine. The other solution is to move the SemanticContext outside of CompilerInvocation, but I am not sure of the implications. Would that be possible? |
@banach-space, alternatively, would it be OK to move the llvm::TargetMachine into the |
Interesting that you mention that - llvm-project/flang/include/flang/Frontend/CompilerInvocation.h Lines 90 to 91 in 79b0330 That's not where I was expecting to find it :) It sounds like things would be more seamless for both of us if I moved EDIT |
Preliminary patch to change lowering/code generation to use llvm::DataLayout information instead of generating "sizeof" GEP (see llvm#71507). Fortran Semantic analysis needs to know about the target type size and alignment to deal with common blocks, and intrinsics like C_SIZEOF/TRANSFER. This information should be obtained from the llvm::DataLayout so that it is consistent during the whole compilation flow. This change is changing flang-new and bbc drivers to: 1. Create the llvm::TargetMachine so that the data layout of the target can be obtained before semantics. 2. Sharing bbc/flang-new set-up of the SemanticConstext.targetCharateristics from the llvm::TargetMachine. For now, the actual part that set-up the Fortran type size and alignment from the llvm::DataLayout is left TODO so that this change is mostly an NFC impacting the drivers. 3. Let the lowering bridge set-up the mlir::Module datalayout attributes since it is doing it for the target attribute, and that allows the llvm data layout information to be available during lowering. As a consequence, LLVM targets must be registered when running semantics, and it is not possible to run semantics for a target that is not registered with the -triple option (hence the power pc specific modules can only be built if the PowerPC target is available).
Co-authored-by: Slava Zakharin <szakharin@nvidia.com>
jeanPerier
force-pushed
the
jpr-datalayout-driver-update-2
branch
from
8c75f6d
to
7b7bac8
Compare
@banach-space, I just rebased on a main with your patch and moved the target machine into |
Yeah, this is looking much better now, thanks! Most importantly, it does not require such a big refactor and leaves a lot of things in the driver as an implementation detail (that could be refined independently). I will go over the finer details later today or on Monday, but definitely won't be making any "large" requests any more - this is heading in the right direction. |
|
The driver changes LGTM! The stuff that's been added to CompilerInstance.cpp (e.g. Thank you for addressing my comments 🙏🏻 |
After llvm#73301, all semantics tests using `triple XXX` options need to have a `REQUIRED: XX-registered-target` since the llvm::TargetMachine is needed to get the llvm::DataLayout before semantics. Fix three tests that lacked this. Fixes: https://lab.llvm.org/buildbot/#/builders/21/builds/87263 https://lab.llvm.org/buildbot/#/builders/268/builds/3841
After #73301, all semantics tests using `triple XXX` options need to have a `REQUIRED: XX-registered-target` since the llvm::TargetMachine is needed to get the llvm::DataLayout before semantics. Fix three tests that lacked this. Fixes: https://lab.llvm.org/buildbot/#/builders/21/builds/87263 https://lab.llvm.org/buildbot/#/builders/268/builds/3841
Preliminary patch to change lowering/code generation to use llvm::DataLayout information instead of generating "sizeof" GEP (see #71507).
Fortran Semantic analysis needs to know about the target type size and alignment to deal with common blocks, and intrinsics like C_SIZEOF/TRANSFER. This information should be obtained from the llvm::DataLayout so that it is consistent during the whole compilation flow.
This change is changing flang-new and bbc drivers to:
For flang-new, the changes are code shuffling: the
llvm::TargetMachineinstance is moved toCompilerInvocationclass so that it can be used to set-up the semantic contexts.setMLIRDataLayoutis moved toflang/Optimizer/Support/DataLayout.h(it will need to be used from codegen pass for fir-opt target independent testing.)), and the code setting-up semantics targetCharacteristics is moved toTools/TargetSetup.hso that it can be shared with bbc.As a consequence, LLVM targets must be registered when running semantics, and it is not possible to run semantics for a target that is not registered with the -triple option (hence the power pc specific modules can only be built if the PowerPC target is available, @DanielCChen and @kkwli , I think I got it right, but you may want to double check the
tools/f18/CMakeLists.txtchange).