[Flang][Driver]Add datalayout before doing LLVM-IR transformation

[Leporacanthicus]

Summary

The earlier available datalyaout allows MLIR to LLVM-IR transformation
to use the datalayout for decisions, such as comparing sizes for
different types of integers.

This should solve https://github.com/llvm/llvm-project/issues/57230

[awarzynski]

Thanks for implementing this @MatsPetersson !

Could add a test to demonstrate the behavior when the input MLIR module does and does not define the data layout?

flang/lib/Frontend/FrontendActions.cpp

590

The comment in FrontendActions.h is a bit confusing:

/// Sets up LLVM's TargetMachine, configures llvmModule accordingly.

With your changes, llvmModule may or may not be configured. And even without your changes, the data layout (extracted from the target machine), is set after the call to setUpTargetMachine (so the LLVM module is not configured accordingly).

I would update this method to focus on "setting up the target machine". And the bit concerned with setting up the triple should probably be extracted.

flang/test/Driver/pic-flags.f90

2

Unrelated change

17

Unrelated change

flang/unittests/Frontend/FrontendActionTest.cpp

181–185

Why is this suddenly needed?

[vzakhari]

flang/lib/Frontend/FrontendActions.cpp

719

I am not sure if we can use string representation of LLVM data layout as MLIR data layout. I would rather follow the Polygeist way and translate LLVM data layout using this method.

753

Do we need this, if MLIR module already has proper data layout? Shouldn't it be translated into LLVM data layout during translation to LLVM IR dialect?

[Leporacanthicus]

flang/lib/Frontend/FrontendActions.cpp

719

Good point. I am trying to test this thoroughly, but I have been busy with other things.

753

Yes. if someone feeds flang-new with a .ll file, it will need to set the DL for LLVM to use later. Because we never created an MLIR module. Hence the check for "is it default" before setting it - we only set it if it wasn't already set.

flang/test/Driver/pic-flags.f90

2

No. We need all the targets that the test runs to to be able to run - now that we set the DL earlier, it fails, where before it didn't.

flang/unittests/Frontend/FrontendActionTest.cpp

181–185

Because when we set the DataLayout earlier, the TargetMachine needs to exist for it to be able to set. It took me a while to understand, but it was clearly the problem, as I discovered once I realized the difference between this test [before these lines were added] and the next one below. We tried to get the TargetMachine, and the LLVM infrastructure has an empty list of target machines, retuns NULL [or end() from some storage]

Previously, we only set DataLayout before turning the LLVM-IR into machine-code, which doesn't happen during this test.

[awarzynski]

Thanks for the clarifications, Mats! I see that with this change you are also making sure that the datalayout is set before the -emit-llvm actions ends. I think that technically that's not required for this change to work - or am I still missing something? In either case, it's not too much noise and we can keep here. But it would be good to point out in the commit message that with this change, flang-new -emit-llvm -S file.f90 will generate an LLVM IR file with correct datalayout present. I hope that I got it right :)

flang/lib/Frontend/FrontendActions.cpp

753

@Leporacanthicus This makes sense, but the name of method that you are using here does not convey that (not your fault!) :) If this was something like this then the intent would be clearer:

if (!llvmModule->getDataLayout())

So, I suggest adding a comment to explain that isDefault() is equivalent to "there is no data layout yet" :)

[Leporacanthicus]

Updates based on review comments:

• Additional MLIR DataLayout.
• Add check lines for datalayout in the MLIR and LLVM output

[awarzynski]

flang/lib/Frontend/FrontendActions.cpp

170–174

Added some suggestion inline (spiting the logic a bit and adding missing comment)

In general, this feels a bit weird. If the lowering bridge provides the MLIR module then it should also configure it correctly, right? i.e. set-up the datalayout. I don't mind this being added here, just thinking about the overall design.

586–587

Why is it not safe to reset it?

I think that it's a good idea to only set it once per compilation. Unless there's a good reasons to allow it multiple times, but I'm not aware of a case requiring that.

754

What happens when consuming an LLVM IR module? In particular, there are 4 scenarios:

• input module contains DL, compiler invocation contains --target=<target>
• input module contains DL, compiler invocation does not contain --target=<target>
• input module does not contain DL, compiler invocation contains --target=<target>
• input module does not contain DL, compiler invocation does not contain --target=<target>

Is the behavior "before" and "after" this change identical? And is it consistent with Clang? I feel that this particular change might have changed the behavior of flang-new.

[Leporacanthicus]

Updates per review comments and local testing.

Mainly, set DataLayout for more conditions, rather than skipping the setting.

flang/lib/Frontend/FrontendActions.cpp

170–174

I looked at this, but it's rather complicated, there isn't enough infrastructure in place to do this.

I plan on raising a ticket for "MLIR support for DataLayout is lacking" (or something like that), but I'd like to have this done first.

586–587

Safe in this case is "doesn't unnecessarily set things that we've already set". I'll reword.

Anyway, turns out we want to set it every time, so no longer there... :)

719

So, it turns out we need both the translation and setting the LLVM datalayout, otherwise we can't "recover" the original layout.

As mentioned above, I intend to raise a ticket for "insufficient DL support in MLIR".

754

Comparing with clang, it behaves the same (except for using SSE instructions in clang, and not in Flang - I don't believe that's to do with my changes tho'). I would like to know what it is, actually, since it probably wouldn't hurt to have in Flang - on the other hand, x86-32 isn't a key target afaik.

Either way, it matches the previous implementation after I tested things.

[vzakhari]

flang/lib/Frontend/FrontendActions.cpp

762

typo: teh

763

typo: differen

[vzakhari]

I accept this (module the typos), but please make sure @awarzynski is okay with the changes. Thank you!

[awarzynski]

flang/lib/Frontend/FrontendActions.cpp

139

Do you need this temporary?

170–174

I looked at this, but it's rather complicated, there isn't enough infrastructure in place to do this.

This is only addressing my 2nd comment in the box above (starting with "In general"). How about the earlier bit? Amongst other things, this comment is incomplete:

// Fetch module from lb, so we can set

I've also made some suggestions inline.

754

Why? :) This comment currently repeats "what" the code does. But it's hard to understand the "why" behind it. Ultimately, we need clear documentation for where the triple comes from.

IIRC, tm is a member of CompilerInvocation. So, tm contains the triple specified on the command line (or the default triple if no triple is specified). And in this block, you are making sure that it's the set-up from CompilerInvocaiton is used to set-up the triple.

This is, IMHO, key. The triple can come from 2 places (3 if you count MLIR modules) and I would like for the code to clearly document this.

flang/test/Driver/pic-flags.f90

1

Unrelated change, please extract to a separate patch.

[Leporacanthicus]

Update comments (clarify/reword, fix typos) and rebase.

[awarzynski]

Thank you @Leporacanthicus , this has been a tricky one! LGTM

flang/include/flang/Frontend/FrontendActions.h

 /// and machine code). Every action that inherits from this class will at
 /// least run the prescanning, parsing, semantic checks and lower the parse
 /// tree to an MLIR module.
 class CodeGenAction : public FrontendAction {
 
   void executeAction() override;
   /// Runs prescan, parsing, sema and lowers to MLIR.
   bool beginSourceFileAction() override;
-  /// Sets up LLVM's TargetMachine, configures llvmModule accordingly.
+  /// Sets up LLVM's TargetMachine.
   void setUpTargetMachine();
   /// Runs the optimization (aka middle-end) pipeline on the LLVM module
   /// associated with this action.
   void runOptimizationPipeline(llvm::raw_pwrite_stream &os);
 
 protected:
   CodeGenAction(BackendActionTy act) : action{act} {};
   /// @name MLIR

flang/include/flang/Optimizer/Support/InitFIR.h

 
 namespace fir::support {
 
 #define FLANG_NONCODEGEN_DIALECT_LIST                                          \
   mlir::AffineDialect, FIROpsDialect, mlir::acc::OpenACCDialect,               \
       mlir::omp::OpenMPDialect, mlir::scf::SCFDialect,                         \
       mlir::arith::ArithDialect, mlir::cf::ControlFlowDialect,                 \
       mlir::func::FuncDialect, mlir::vector::VectorDialect,                    \
-      mlir::math::MathDialect
+      mlir::math::MathDialect, mlir::DLTIDialect
 
 // The definitive list of dialects used by flang.
 #define FLANG_DIALECT_LIST                                                     \
   FLANG_NONCODEGEN_DIALECT_LIST, FIRCodeGenDialect, mlir::LLVM::LLVMDialect
 
 inline void registerNonCodegenDialects(mlir::DialectRegistry &registry) {
   registry.insert<FLANG_NONCODEGEN_DIALECT_LIST>();
 }

flang/lib/Frontend/CMakeLists.txt

add_flang_library(flangFrontend
   FIRDialect
   FIRSupport
   FIRBuilder
   FIRCodeGen
   FIRTransforms
   MLIRTransforms
   MLIRLLVMToLLVMIRTranslation
   MLIRSCFToControlFlow
+  MLIRTargetLLVMIRImport
   ${dialect_libs}
 
   LINK_COMPONENTS
   Passes
   Analysis
   IRReader
   Option
   Support
   Target
   FrontendOpenACC
   FrontendOpenMP
 )

flang/lib/Frontend/FrontendActions.cpp

 #include "flang/Parser/unparse.h"
 #include "flang/Semantics/runtime-type-info.h"
 #include "flang/Semantics/semantics.h"
 #include "flang/Semantics/unparse-with-symbols.h"
 
 #include "mlir/IR/Dialect.h"
 #include "mlir/Parser/Parser.h"
 #include "mlir/Pass/PassManager.h"
+#include "mlir/Support/LLVM.h"
+#include "mlir/Target/LLVMIR/Import.h"
 #include "mlir/Target/LLVMIR/ModuleTranslation.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticFrontend.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Bitcode/BitcodeWriterPass.h"
 #include "llvm/IR/LegacyPassManager.h"
bool PrescanAndSemaDebugAction::beginSourceFileAction() {
   // semantic checks are made to succeed unconditionally to prevent this action
   // from exiting early (i.e. in the presence of semantic errors). We should
   // never do this in actions intended for end-users or otherwise regular
   // compiler workflows!
   return runPrescan() && runParse() && (runSemanticChecks() || true) &&
          (generateRtTypeTables() || true);
 }
 
+static void setMLIRDataLayout(mlir::ModuleOp &mlirModule,
+                              const llvm::DataLayout &dl) {
+  mlir::MLIRContext *context = mlirModule.getContext();
+  mlirModule->setAttr(
+      mlir::LLVM::LLVMDialect::getDataLayoutAttrName(),
+      mlir::StringAttr::get(context, dl.getStringRepresentation()));
+  mlir::DataLayoutSpecInterface dlSpec = mlir::translateDataLayout(dl, context);
+  mlirModule->setAttr(mlir::DLTIDialect::kDataLayoutAttrName, dlSpec);
+}
+
 bool CodeGenAction::beginSourceFileAction() {
   llvmCtx = std::make_unique<llvm::LLVMContext>();
   CompilerInstance &ci = this->getInstance();
 
   // If the input is an LLVM file, just parse it and return.
   if (this->getCurrentInput().getKind().getLanguage() == Language::LLVM_IR) {
     llvm::SMDiagnostic err;
     llvmModule = llvm::parseIRFile(getCurrentInput().getFile(), err, *llvmCtx);
if (this->getCurrentInput().getKind().getLanguage() == Language::MLIR) {
     if (!module || mlir::failed(module->verifyInvariants())) {
       unsigned diagID = ci.getDiagnostics().getCustomDiagID(
           clang::DiagnosticsEngine::Error, "Could not parse FIR");
       ci.getDiagnostics().Report(diagID);
       return false;
     }
 
     mlirModule = std::make_unique<mlir::ModuleOp>(module.release());
+    setUpTargetMachine();
+    const llvm::DataLayout &dl = tm->createDataLayout();
+    setMLIRDataLayout(*mlirModule, dl);
     return true;
   }
 
   // Otherwise, generate an MLIR module from the input Fortran source
   if (getCurrentInput().getKind().getLanguage() != Language::Fortran) {
     unsigned diagID = ci.getDiagnostics().getCustomDiagID(
         clang::DiagnosticsEngine::Error,
         "Invalid input type - expecting a Fortran file");
bool CodeGenAction::beginSourceFileAction() {
   lower::LoweringBridge lb = Fortran::lower::LoweringBridge::create(
       *mlirCtx, ci.getInvocation().getSemanticsContext(), defKinds,
       ci.getInvocation().getSemanticsContext().intrinsics(),
       ci.getInvocation().getSemanticsContext().targetCharacteristics(),
       ci.getParsing().allCooked(), ci.getInvocation().getTargetOpts().triple,
       kindMap, ci.getInvocation().getLoweringOpts(),
       ci.getInvocation().getFrontendOpts().envDefaults);
 
+  // Fetch module from lb, so we can set
+  mlirModule = std::make_unique<mlir::ModuleOp>(lb.getModule());
+  setUpTargetMachine();
+  const llvm::DataLayout &dl = tm->createDataLayout();
+  setMLIRDataLayout(*mlirModule, dl);
+
   // Create a parse tree and lower it to FIR
   Fortran::parser::Program &parseTree{*ci.getParsing().parseTree()};
   lb.lower(parseTree, ci.getInvocation().getSemanticsContext());
-  mlirModule = std::make_unique<mlir::ModuleOp>(lb.getModule());
 
   // run the default passes.
   mlir::PassManager pm(mlirCtx.get(), mlir::OpPassManager::Nesting::Implicit);
   pm.enableVerifier(/*verifyPasses=*/true);
   pm.addPass(std::make_unique<Fortran::lower::VerifierPass>());
 
   if (mlir::failed(pm.run(*mlirModule))) {
     unsigned diagID = ci.getDiagnostics().getCustomDiagID(
getCGOptLevel(const Fortran::frontend::CodeGenOptions &opts) {
   case 2:
     return llvm::CodeGenOpt::Default;
   case 3:
     return llvm::CodeGenOpt::Aggressive;
   }
 }
 
 void CodeGenAction::setUpTargetMachine() {
   CompilerInstance &ci = this->getInstance();
 
-  // Set the triple based on the CompilerInvocation set-up
   const std::string &theTriple = ci.getInvocation().getTargetOpts().triple;
-  if (llvmModule->getTargetTriple() != theTriple) {
-    ci.getDiagnostics().Report(clang::diag::warn_fe_override_module)
-        << theTriple;
-    llvmModule->setTargetTriple(theTriple);
-  }
 
   // Create `Target`
   std::string error;
   const llvm::Target *theTarget =
       llvm::TargetRegistry::lookupTarget(theTriple, error);
   assert(theTarget && "Failed to create Target");
 
   // Create `TargetMachine`
   const auto &CGOpts = ci.getInvocation().getCodeGenOpts();
   llvm::CodeGenOpt::Level OptLevel = getCGOptLevel(CGOpts);
void CodeGenAction::runOptimizationPipeline(llvm::raw_pwrite_stream &os) {
   // Run the passes.
   mpm.run(*llvmModule, mam);
 }
 
 void CodeGenAction::executeAction() {
   CompilerInstance &ci = this->getInstance();
 
   // If the output stream is a file, generate it and define the corresponding
   // output stream. If a pre-defined output stream is available, we will use
   // that instead.
   //
   // NOTE: `os` is a smart pointer that will be destroyed at the end of this
   // method. However, it won't be written to until `codeGenPasses` is
   // destroyed. By defining `os` before `codeGenPasses`, we make sure that the
   // output stream won't be destroyed before it is written to. This only
   // applies when an output file is used (i.e. there is no pre-defined output
   // stream).
if (action == BackendActionTy::Backend_EmitMLIR) {
     return;
   }
 
   // Generate an LLVM module if it's not already present (it will already be
   // present if the input file is an LLVM IR/BC file).
   if (!llvmModule)
     generateLLVMIR();
 
+  setUpTargetMachine();
+
+  // Set the triple based on the targetmachine
+  const std::string &theTriple = tm->getTargetTriple().str();
+
+  if (llvmModule->getTargetTriple() != theTriple) {
+    ci.getDiagnostics().Report(clang::diag::warn_fe_override_module)
+        << theTriple;
+  }
+  // Always set the triple and data layout, to make sure they match and are set.
+  // Note that this overwrites any datalayout stored in teh LLVM-IR. This helps
+  // when using IR originally created for a differen target, e.g. x86-64 target
+  // IR being turned to for i386 target. Incompatible DL will cause an assert.
+  llvmModule->setTargetTriple(theTriple);
+  llvmModule->setDataLayout(tm->createDataLayout());
+
   // Run LLVM's middle-end (i.e. the optimizer).
   runOptimizationPipeline(*os);
 
   if (action == BackendActionTy::Backend_EmitLL) {
     llvmModule->print(ci.isOutputStreamNull() ? *os : ci.getOutputStream(),
                       /*AssemblyAnnotationWriter=*/nullptr);
     return;
   }
 
-  setUpTargetMachine();
-  llvmModule->setDataLayout(tm->createDataLayout());
-
   if (action == BackendActionTy::Backend_EmitBC) {
     // This action has effectively been completed in runOptimizationPipeline.
     return;
   }
 
   // Run LLVM's backend and generate either assembly or machine code
   if (action == BackendActionTy::Backend_EmitAssembly ||
       action == BackendActionTy::Backend_EmitObj) {

flang/test/Driver/emit-llvm.f90

 ! Test the `-emit-llvm` option
 
 ! UNSUPPORTED: system-windows
 ! Windows is currently not supported in flang/lib/Optimizer/CodeGen/Target.cpp
 
 ! RUN: %flang_fc1 -emit-llvm %s -o - | FileCheck %s
 
 ! CHECK: ; ModuleID = 'FIRModule'
+! CHECK: target datalayout =
 ! CHECK: define void @_QQmain()
 ! CHECK-NEXT:  ret void
 ! CHECK-NEXT: }
 
 end program

flang/test/Driver/emit-mlir.f90

 ! Test the `-emit-mlir` option
 
 ! RUN: %flang_fc1 -emit-mlir %s -o - | FileCheck %s
 ! RUN: %flang_fc1 -emit-fir %s -o - | FileCheck %s
 
 ! Verify that an `.mlir` file is created when `-emit-mlir` is used. Do it in a temporary directory, which will be cleaned up by the
 ! LIT runner.
 ! RUN: rm -rf %t-dir && mkdir -p %t-dir && cd %t-dir
 ! RUN: cp %s .
 ! RUN: %flang_fc1 -emit-mlir emit-mlir.f90 && ls emit-mlir.mlir
 
 ! CHECK: module attributes {
+! CHECK-SAME: dlti.dl_spec =
+! CHECK-SAME: llvm.data_layout =
 ! CHECK-LABEL: func @_QQmain() {
 ! CHECK-NEXT:  return
 ! CHECK-NEXT: }
 ! CHECK-NEXT: fir.global @_QQEnvironmentDefaults constant : !fir.ref<tuple<i[[int_size:.*]], !fir.ref<!fir.array<0xtuple<!fir.ref<i8>, !fir.ref<i8>>>>>> {
 ! CHECK-NEXT:  %[[VAL_0:.*]] = fir.zero_bits !fir.ref<tuple<i[[int_size]], !fir.ref<!fir.array<0xtuple<!fir.ref<i8>, !fir.ref<i8>>>>>>
 ! CHECK-NEXT: fir.has_value  %[[VAL_0]] : !fir.ref<tuple<i[[int_size]], !fir.ref<!fir.array<0xtuple<!fir.ref<i8>, !fir.ref<i8>>>>>>
 ! CHECK-NEXT: }
 ! CHECK-NEXT: }
 
 end program

flang/test/Driver/pic-flags.f90

+! REQUIRES: aarch64-registered-target && x86-registered-target && arm-registered-target
 ! RUN: %flang -v -S -emit-llvm -o - %s --target=aarch64-linux-gnu -fno-pie 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-STATIC,CHECK-STATIC-IR
 
 ! RUN: %flang -v -S -emit-llvm -o - %s --target=aarch64-linux-gnu 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-PIE-LEVEL2,CHECK-PIE-LEVEL2-IR
 ! RUN: %flang -v -S -emit-llvm -o - %s --target=aarch64-linux-gnu -fpie 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-PIE-LEVEL1,CHECK-PIE-LEVEL1-IR
 ! RUN: %flang -v -S -emit-llvm -o - %s --target=aarch64-linux-gnu -fPIE 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-PIE-LEVEL2,CHECK-PIE-LEVEL2-IR
 
 ! RUN: %flang -v -S -emit-llvm -o - %s --target=aarch64-linux-gnu -fpic 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-PIC-LEVEL1,CHECK-PIC-LEVEL1-IR
 ! RUN: %flang -v -S -emit-llvm -o - %s --target=aarch64-linux-gnu -fPIC 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-PIC-LEVEL2,CHECK-PIC-LEVEL2-IR
 
 ! RUN: %flang -v -### -o - %s --target=i386-apple-darwin -mdynamic-no-pic 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-DYNAMIC-NO-PIC-32
 ! RUN: %flang -v -### -o - %s --target=x86_64-apple-darwin -mdynamic-no-pic 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-DYNAMIC-NO-PIC-64
 
 ! RUN: %flang -v -### -o - %s --target=arm-none-eabi -fropi 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-ROPI
 ! RUN: %flang -v -### -o - %s --target=arm-none-eabi -frwpi 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-RWPI
 ! RUN: %flang -v -### -o - %s --target=arm-none-eabi -fropi -frwpi 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-ROPI-RWPI
 
-
 ! CHECK: -fc1
 
 
 !! -fno-pie.
 ! CHECK-STATIC: -mrelocation-model static
 ! CHECK-STATIC-NOT: -pic
 
 ! CHECK-STATIC-IR-NOT: {{PIE|PIC}} Level

flang/unittests/Frontend/FrontendActionTest.cpp

TEST_F(FrontendActionTest, EmitLLVM) {
 
   // Set-up the action kind.
   compInst.getInvocation().getFrontendOpts().programAction = EmitLLVM;
 
   // Set-up default target triple.
   compInst.getInvocation().getTargetOpts().triple =
       llvm::Triple::normalize(llvm::sys::getDefaultTargetTriple());
 
+  // Initialise LLVM backend
+  llvm::InitializeAllTargets();
+  llvm::InitializeAllTargetMCs();
+  llvm::InitializeAllAsmPrinters();
+
   // Set-up the output stream. We are using output buffer wrapped as an output
   // stream, as opposed to an actual file (or a file descriptor).
   llvm::SmallVector<char> outputFileBuffer;
   std::unique_ptr<llvm::raw_pwrite_stream> outputFileStream(
       new llvm::raw_svector_ostream(outputFileBuffer));
   compInst.setOutputStream(std::move(outputFileStream));
 
   // Execute the action.