[IRParser] Pass a const Module& reference to the DataLayout callback

[arichardson]

Summary

This allows the "override DataLayout" callback to read both the
DataLayout and TargetTriple that was defined in the IR module when
inferring the default (for example we might not want to override the
existing value if it's non-empty).
We could also add an additional argument instead, but I feel that
passing a const reference to the Module is simpler and more extensible.

[arichardson]

@mehdi_amini In D78403 you were against passing a Module* callback, is a read-only const Module& okay or should I pass DataLayout + triple instead? I prefer the former, but if there are strong objections I'm happy to change it to the latter. Passing a read-only module should make it easier to extend in the future if needed.

[mehdi_amini]

In general I'm in favor of narrow API: that is pass what you need and not more. That makes it for cleaner API contracts which are easier to understand and/or maintain.

[arichardson]

Looks like this is obsolete now that D140985 landed, will rebase the remaining patch stack.

llvm/include/llvm/AsmParser/LLParser.h

public:
     LLParser(StringRef F, SourceMgr &SM, SMDiagnostic &Err, Module *M,
              ModuleSummaryIndex *Index, LLVMContext &Context,
              SlotMapping *Slots = nullptr)
         : Context(Context), OPLex(F, SM, Err, Context),
           Lex(F, SM, Err, Context), M(M), Index(Index), Slots(Slots),
           BlockAddressPFS(nullptr) {}
     bool Run(
         bool UpgradeDebugInfo, DataLayoutCallbackTy DataLayoutCallback =
-                                   [](StringRef) { return std::nullopt; });
+                                   [](const Module &) { return std::nullopt; });
 
     bool parseStandaloneConstantValue(Constant *&C, const SlotMapping *Slots);
 
     bool parseTypeAtBeginning(Type *&Ty, unsigned &Read,
                               const SlotMapping *Slots);
 
     LLVMContext &getContext() { return Context; }
 

llvm/include/llvm/AsmParser/Parser.h

 class LLVMContext;
 class MemoryBufferRef;
 class Module;
 class ModuleSummaryIndex;
 struct SlotMapping;
 class SMDiagnostic;
 class Type;
 
-typedef llvm::function_ref<std::optional<std::string>(StringRef)>
+typedef llvm::function_ref<std::optional<std::string>(const Module &)>
     DataLayoutCallbackTy;
 
 /// This function is a main interface to the LLVM Assembly Parser. It parses
 /// an ASCII file that (presumably) contains LLVM Assembly code. It returns a
 /// Module (intermediate representation) with the corresponding features. Note
 /// that this does not verify that the generated Module is valid, so you should
 /// run the verifier after parsing the file to check that it is okay.
 /// Parse LLVM Assembly from a file

 /// \param Err Error result info.
 /// \param Context Context in which to allocate globals info.
 /// \param Slots The optional slot mapping that will be initialized during
 ///              parsing.
 /// \param DataLayoutCallback Override datalayout in the llvm assembly.
 ParsedModuleAndIndex parseAssemblyFileWithIndex(
     StringRef Filename, SMDiagnostic &Err, LLVMContext &Context,
     SlotMapping *Slots = nullptr,
-    DataLayoutCallbackTy DataLayoutCallback = [](StringRef) {
+    DataLayoutCallbackTy DataLayoutCallback = [](const Module &) {
       return std::nullopt;
     });
 
 /// Only for use in llvm-as for testing; this does not produce a valid module.
 ParsedModuleAndIndex parseAssemblyFileWithIndexNoUpgradeDebugInfo(
     StringRef Filename, SMDiagnostic &Err, LLVMContext &Context,
     SlotMapping *Slots, DataLayoutCallbackTy DataLayoutCallback);
 

 /// \param F The MemoryBuffer containing assembly
 /// \param Err Error result info.
 /// \param Slots The optional slot mapping that will be initialized during
 ///              parsing.
 /// \param DataLayoutCallback Override datalayout in the llvm assembly.
 std::unique_ptr<Module> parseAssembly(
     MemoryBufferRef F, SMDiagnostic &Err, LLVMContext &Context,
     SlotMapping *Slots = nullptr,
-    DataLayoutCallbackTy DataLayoutCallback = [](StringRef) {
+    DataLayoutCallbackTy DataLayoutCallback = [](const Module &) {
       return std::nullopt;
     });
 
 /// Parse LLVM Assembly including the summary index from a MemoryBuffer.
 ///
 /// \param F The MemoryBuffer containing assembly with summary
 /// \param Err Error result info.
 /// \param Slots The optional slot mapping that will be initialized during

 /// \param Err Error result info.
 /// \param Slots The optional slot mapping that will be initialized during
 ///              parsing.
 /// \return true on error.
 /// \param DataLayoutCallback Override datalayout in the llvm assembly.
 bool parseAssemblyInto(
     MemoryBufferRef F, Module *M, ModuleSummaryIndex *Index, SMDiagnostic &Err,
     SlotMapping *Slots = nullptr,
-    DataLayoutCallbackTy DataLayoutCallback = [](StringRef) {
+    DataLayoutCallbackTy DataLayoutCallback = [](const Module &) {
       return std::nullopt;
     });
 
 /// Parse a type and a constant value in the given string.
 ///
 /// The constant value can be any LLVM constant, including a constant
 /// expression.
 ///

llvm/include/llvm/Bitcode/BitcodeReader.h

 #include <vector>
 namespace llvm {
 
 class LLVMContext;
 class Module;
 class MemoryBuffer;
 class ModuleSummaryIndex;
 
-typedef llvm::function_ref<std::optional<std::string>(StringRef)>
+typedef llvm::function_ref<std::optional<std::string>(const Module &)>
     DataLayoutCallbackTy;
 
-  // These functions are for converting Expected/Error values to
-  // ErrorOr/std::error_code for compatibility with legacy clients. FIXME:
-  // Remove these functions once no longer needed by the C and libLTO APIs.
 
-  std::error_code errorToErrorCodeAndEmitErrors(LLVMContext &Ctx, Error Err);
 
-  template <typename T>
-  ErrorOr<T> expectedToErrorOrAndEmitErrors(LLVMContext &Ctx, Expected<T> Val) {
-    if (!Val)
-      return errorToErrorCodeAndEmitErrors(Ctx, Val.takeError());
-    return std::move(*Val);
   }
 
   struct BitcodeFileContents;
 
   /// Basic information extracted from a bitcode module to be used for LTO.
   struct BitcodeLTOInfo {
     bool IsThinLTO;
     bool HasSummary;
public:
     /// importing into another module.
     Expected<std::unique_ptr<Module>> getLazyModule(LLVMContext &Context,
                                                     bool ShouldLazyLoadMetadata,
                                                     bool IsImporting);
 
     /// Read the entire bitcode module and return it.
     Expected<std::unique_ptr<Module>> parseModule(
         LLVMContext &Context, DataLayoutCallbackTy DataLayoutCallback =
-                                  [](StringRef) { return std::nullopt; });
+                                  [](const Module &) { return std::nullopt; });
 
     /// Returns information about the module to be used for LTO: whether to
     /// compile with ThinLTO, and whether it has a summary.
     Expected<BitcodeLTOInfo> getLTOInfo();
 
     /// Parse the specified bitcode buffer, returning the module summary index.
     Expected<std::unique_ptr<ModuleSummaryIndex>> getSummary();
 
ErrorOr<T> expectedToErrorOrAndEmitErrors(LLVMContext &Ctx, Expected<T> Val) {
   /// Read the header of the specified bitcode buffer and extract just the
   /// producer string information. If successful, this returns a string. On
   /// error, this returns "".
   Expected<std::string> getBitcodeProducerString(MemoryBufferRef Buffer);
 
   /// Read the specified bitcode file, returning the module.
   Expected<std::unique_ptr<Module>> parseBitcodeFile(
       MemoryBufferRef Buffer, LLVMContext &Context,
-      DataLayoutCallbackTy DataLayoutCallback = [](StringRef) {
+      DataLayoutCallbackTy DataLayoutCallback = [](const Module &) {
         return std::nullopt;
       });
 
   /// Returns LTO information for the specified bitcode file.
   Expected<BitcodeLTOInfo> getBitcodeLTOInfo(MemoryBufferRef Buffer);
 
   /// Parse the specified bitcode buffer, returning the module summary index.
   Expected<std::unique_ptr<ModuleSummaryIndex>>

llvm/include/llvm/CodeGen/MIRParser/MIRParser.h

 class LLVMContext;
 class MemoryBuffer;
 class Module;
 class MIRParserImpl;
 class MachineModuleInfo;
 class SMDiagnostic;
 class StringRef;
 
-typedef llvm::function_ref<std::optional<std::string>(StringRef)>
+typedef llvm::function_ref<std::optional<std::string>(const Module &)>
     DataLayoutCallbackTy;
 
 /// This class initializes machine functions by applying the state loaded from
 /// a MIR file.
 class MIRParser {
   std::unique_ptr<MIRParserImpl> Impl;
 
 public:
   MIRParser(std::unique_ptr<MIRParserImpl> Impl);
   MIRParser(const MIRParser &) = delete;
   ~MIRParser();
 
   /// Parses the optional LLVM IR module in the MIR file.
   ///
   /// A new, empty module is created if the LLVM IR isn't present.
   /// \returns nullptr if a parsing error occurred.
   std::unique_ptr<Module>
-  parseIRModule(DataLayoutCallbackTy DataLayoutCallback = [](StringRef) {
+  parseIRModule(DataLayoutCallbackTy DataLayoutCallback = [](const Module &) {
     return std::nullopt;
   });
 
   /// Parses MachineFunctions in the MIR file and add them to the given
   /// MachineModuleInfo \p MMI.
   ///
   /// \returns true if an error occurred.
   bool parseMachineFunctions(Module &M, MachineModuleInfo &MMI);

llvm/include/llvm/IRReader/IRReader.h

 namespace llvm {
 
 class MemoryBuffer;
 class MemoryBufferRef;
 class Module;
 class SMDiagnostic;
 class LLVMContext;
 
-typedef llvm::function_ref<std::optional<std::string>(StringRef)>
+typedef llvm::function_ref<std::optional<std::string>(const Module &)>
     DataLayoutCallbackTy;
 
 /// If the given MemoryBuffer holds a bitcode image, return a Module
 /// for it which does lazy deserialization of function bodies.  Otherwise,
 /// attempt to parse it as LLVM Assembly and return a fully populated
 /// Module. The ShouldLazyLoadMetadata flag is passed down to the bitcode
 /// reader to optionally enable lazy metadata loading. This takes ownership
 /// of \p Buffer.
getLazyIRFileModule(StringRef Filename, SMDiagnostic &Err, LLVMContext &Context,
                     bool ShouldLazyLoadMetadata = false);
 
 /// If the given MemoryBuffer holds a bitcode image, return a Module
 /// for it.  Otherwise, attempt to parse it as LLVM Assembly and return
 /// a Module for it.
 /// \param DataLayoutCallback Override datalayout in the llvm assembly.
 std::unique_ptr<Module> parseIR(
     MemoryBufferRef Buffer, SMDiagnostic &Err, LLVMContext &Context,
-    DataLayoutCallbackTy DataLayoutCallback = [](StringRef) {
+    DataLayoutCallbackTy DataLayoutCallback = [](const Module &) {
       return std::nullopt;
     });
 
 /// If the given file holds a bitcode image, return a Module for it.
 /// Otherwise, attempt to parse it as LLVM Assembly and return a Module
 /// for it.
 /// \param DataLayoutCallback Override datalayout in the llvm assembly.
 std::unique_ptr<Module> parseIRFile(
     StringRef Filename, SMDiagnostic &Err, LLVMContext &Context,
-    DataLayoutCallbackTy DataLayoutCallback = [](StringRef) {
+    DataLayoutCallbackTy DataLayoutCallback = [](const Module &) {
       return std::nullopt;
     });
 }
 
 #endif

llvm/lib/AsmParser/LLParser.cpp

if (Context.shouldDiscardValueNames())
     return error(
         Lex.getLoc(),
         "Can't read textual IR with a Context that discards named Values");
 
   if (M) {
     if (parseTargetDefinitions())
       return true;
 
-    if (auto LayoutOverride = DataLayoutCallback(M->getTargetTriple()))
+    if (auto LayoutOverride = DataLayoutCallback(*M))
       M->setDataLayout(*LayoutOverride);
   }
 
   return parseTopLevelEntities() || validateEndOfModule(UpgradeDebugInfo) ||
          validateEndOfIndex();
 }
 
 bool LLParser::parseStandaloneConstantValue(Constant *&C,

llvm/lib/AsmParser/Parser.cpp

 }
 
 ParsedModuleAndIndex llvm::parseAssemblyWithIndex(MemoryBufferRef F,
                                                   SMDiagnostic &Err,
                                                   LLVMContext &Context,
                                                   SlotMapping *Slots) {
   return ::parseAssemblyWithIndex(F, Err, Context, Slots,
                                   /*UpgradeDebugInfo*/ true,
-                                  [](StringRef) { return std::nullopt; });
+                                  [](const Module &) { return std::nullopt; });
 }
 
 static ParsedModuleAndIndex
 parseAssemblyFileWithIndex(StringRef Filename, SMDiagnostic &Err,
                            LLVMContext &Context, SlotMapping *Slots,
                            bool UpgradeDebugInfo,
                            DataLayoutCallbackTy DataLayoutCallback) {
   ErrorOr<std::unique_ptr<MemoryBuffer>> FileOrErr =
static bool parseSummaryIndexAssemblyInto(MemoryBufferRef F,
   SourceMgr SM;
   std::unique_ptr<MemoryBuffer> Buf = MemoryBuffer::getMemBuffer(F);
   SM.AddNewSourceBuffer(std::move(Buf), SMLoc());
 
   // The parser holds a reference to a context that is unused when parsing the
   // index, but we need to initialize it.
   LLVMContext unusedContext;
   return LLParser(F.getBuffer(), SM, Err, nullptr, &Index, unusedContext)
-      .Run(true, [](StringRef) { return std::nullopt; });
+      .Run(true, [](const Module &) { return std::nullopt; });
 }
 
 std::unique_ptr<ModuleSummaryIndex>
 llvm::parseSummaryIndexAssembly(MemoryBufferRef F, SMDiagnostic &Err) {
   std::unique_ptr<ModuleSummaryIndex> Index =
       std::make_unique<ModuleSummaryIndex>(/*HaveGVs=*/false);
 
   if (parseSummaryIndexAssemblyInto(F, *Index, Err))

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

private:
 
   /// Converts alignment exponent (i.e. power of two (or zero)) to the
   /// corresponding alignment to use. If alignment is too large, returns
   /// a corresponding error code.
   Error parseAlignmentValue(uint64_t Exponent, MaybeAlign &Alignment);
   Error parseAttrKind(uint64_t Code, Attribute::AttrKind *Kind);
   Error parseModule(
       uint64_t ResumeBit, bool ShouldLazyLoadMetadata = false,
-      DataLayoutCallbackTy DataLayoutCallback = [](StringRef) {
+      DataLayoutCallbackTy DataLayoutCallback = [](const Module &) {
         return std::nullopt;
       });
 
   Error parseComdatRecord(ArrayRef<uint64_t> Record);
   Error parseGlobalVarRecord(ArrayRef<uint64_t> Record);
   Error parseFunctionRecord(ArrayRef<uint64_t> Record);
   Error parseGlobalIndirectSymbolRecord(unsigned BitCode,
                                         ArrayRef<uint64_t> Record);
auto ResolveDataLayout = [&] {
     // datalayout and triple can't be parsed after this point.
     ResolvedDataLayout = true;
 
     // Upgrade data layout string.
     std::string DL = llvm::UpgradeDataLayoutString(
         TheModule->getDataLayoutStr(), TheModule->getTargetTriple());
     TheModule->setDataLayout(DL);
 
-    if (auto LayoutOverride =
-            DataLayoutCallback(TheModule->getTargetTriple()))
+    if (auto LayoutOverride = DataLayoutCallback(*TheModule))
       TheModule->setDataLayout(*LayoutOverride);
   };
 
   // Read all the records for this module.
   while (true) {
     Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advance();
     if (!MaybeEntry)
       return MaybeEntry.takeError();
BitcodeModule::getModuleImpl(LLVMContext &Context, bool MaterializeAll,
   }
   return std::move(M);
 }
 
 Expected<std::unique_ptr<Module>>
 BitcodeModule::getLazyModule(LLVMContext &Context, bool ShouldLazyLoadMetadata,
                              bool IsImporting) {
   return getModuleImpl(Context, false, ShouldLazyLoadMetadata, IsImporting,
-                       [](StringRef) { return std::nullopt; });
+                       [](const Module &) { return std::nullopt; });
 }
 
 // Parse the specified bitcode buffer and merge the index into CombinedIndex.
 // We don't use ModuleIdentifier here because the client may need to control the
 // module path used in the combined summary (e.g. when reading summaries for
 // regular LTO modules).
 Error BitcodeModule::readSummary(
     ModuleSummaryIndex &CombinedIndex, StringRef ModulePath, uint64_t ModuleId,

llvm/lib/CodeGen/MIRParser/MIRParser.cpp

 std::unique_ptr<Module>
 MIRParserImpl::parseIRModule(DataLayoutCallbackTy DataLayoutCallback) {
   if (!In.setCurrentDocument()) {
     if (In.error())
       return nullptr;
     // Create an empty module when the MIR file is empty.
     NoMIRDocuments = true;
     auto M = std::make_unique<Module>(Filename, Context);
-    if (auto LayoutOverride = DataLayoutCallback(M->getTargetTriple()))
+    if (auto LayoutOverride = DataLayoutCallback(*M))
       M->setDataLayout(*LayoutOverride);
     return M;
   }
 
   std::unique_ptr<Module> M;
   // Parse the block scalar manually so that we can return unique pointer
   // without having to go trough YAML traits.
   if (const auto *BSN =
           dyn_cast_or_null<yaml::BlockScalarNode>(In.getCurrentNode())) {
     SMDiagnostic Error;
     M = parseAssembly(MemoryBufferRef(BSN->getValue(), Filename), Error,
                       Context, &IRSlots, DataLayoutCallback);
     if (!M) {
       reportDiagnostic(diagFromBlockStringDiag(Error, BSN->getSourceRange()));
       return nullptr;
     }
     In.nextDocument();
     if (!In.setCurrentDocument())
       NoMIRDocuments = true;
   } else {
     // Create an new, empty module.
     M = std::make_unique<Module>(Filename, Context);
-    if (auto LayoutOverride = DataLayoutCallback(M->getTargetTriple()))
+    if (auto LayoutOverride = DataLayoutCallback(*M))
       M->setDataLayout(*LayoutOverride);
     NoLLVMIR = true;
   }
   return M;
 }
 
 bool MIRParserImpl::parseMachineFunctions(Module &M, MachineModuleInfo &MMI) {
   if (NoMIRDocuments)

llvm/tools/llc/llc.cpp

static int compileModule(char **argv, LLVMContext &Context) {
   std::optional<Reloc::Model> RM = codegen::getExplicitRelocModel();
   std::optional<CodeModel::Model> CM = codegen::getExplicitCodeModel();
 
   const Target *TheTarget = nullptr;
   std::unique_ptr<TargetMachine> Target;
 
   // If user just wants to list available options, skip module loading
   if (!SkipModule) {
-    auto SetDataLayout =
-        [&](StringRef DataLayoutTargetTriple) -> std::optional<std::string> {
+    auto SetDataLayout = [&](const Module &M) -> std::optional<std::string> {
       // If we are supposed to override the target triple, do so now.
-      std::string IRTargetTriple = DataLayoutTargetTriple.str();
+      // TODO: it would be nice to use codegen::inferDataLayoutOverride() to
+      //  avoid code duplication, but this lambda has additional externally
+      //  visible side effects such as setting up the target and TargetOptions.
+      std::string IRTargetTriple = M.getTargetTriple();
       if (!TargetTriple.empty())
         IRTargetTriple = Triple::normalize(TargetTriple);
       TheTriple = Triple(IRTargetTriple);
       if (TheTriple.getTriple().empty())
         TheTriple.setTriple(sys::getDefaultTargetTriple());
 
       std::string Error;
       TheTarget =

llvm/tools/llvm-as/llvm-as.cpp

 int main(int argc, char **argv) {
   InitLLVM X(argc, argv);
   cl::HideUnrelatedOptions(AsCat);
   cl::ParseCommandLineOptions(argc, argv, "llvm .ll -> .bc assembler\n");
   LLVMContext Context;
 
   // Parse the file now...
   SMDiagnostic Err;
-  auto SetDataLayout = [](StringRef) -> std::optional<std::string> {
+  auto SetDataLayout = [](const Module &) -> std::optional<std::string> {
     if (ClDataLayout.empty())
       return std::nullopt;
     return ClDataLayout;
   };
   ParsedModuleAndIndex ModuleAndIndex;
   if (DisableVerify) {
     ModuleAndIndex = parseAssemblyFileWithIndexNoUpgradeDebugInfo(
         InputFilename, Err, Context, nullptr, SetDataLayout);

llvm/tools/llvm-reduce/ReducerWorkItem.cpp

if (IsMIR) {
     if (std::error_code EC = FileOrErr.getError()) {
       WithColor::error(errs(), ToolName) << EC.message() << '\n';
       return {nullptr, false};
     }
 
     std::unique_ptr<MIRParser> MParser =
         createMIRParser(std::move(FileOrErr.get()), Ctxt);
 
-    auto SetDataLayout =
-        [&](StringRef DataLayoutTargetTriple) -> Optional<std::string> {
+    auto SetDataLayout = [&](const Module &M) -> Optional<std::string> {
       // NB: We always call createTargetMachineForTriple() even if an explicit
       // DataLayout is already set in the module since we want to use this
       // callback to setup the TargetMachine rather than doing it later.
-      std::string IRTargetTriple = DataLayoutTargetTriple.str();
+      std::string IRTargetTriple = M.getTargetTriple();
       if (!TargetTriple.empty())
         IRTargetTriple = Triple::normalize(TargetTriple);
       TheTriple = Triple(IRTargetTriple);
       if (TheTriple.getTriple().empty())
         TheTriple.setTriple(sys::getDefaultTargetTriple());
       ExitOnError ExitOnErr(std::string(ToolName) + ": error: ");
       TM = ExitOnErr(codegen::createTargetMachineForTriple(TheTriple.str()));
-
+      // If an explicit data layout already defined, we do not use the one from
+      // the TargetMachine.
+      if (!M.getDataLayout().isDefault())
+        return None;
       return TM->createDataLayout().getStringRepresentation();
     };
 
     std::unique_ptr<Module> M = MParser->parseIRModule(SetDataLayout);
     LLVMTargetMachine *LLVMTM = static_cast<LLVMTargetMachine *>(TM.get());
 
     MMM->MMI = std::make_unique<MachineModuleInfo>(LLVMTM);
     MParser->parseMachineFunctions(*M, *MMM->MMI);

llvm/tools/opt/opt.cpp

Expected<std::unique_ptr<ToolOutputFile>> RemarksFileOrErr =
                                    RemarksHotnessThreshold);
   if (Error E = RemarksFileOrErr.takeError()) {
     errs() << toString(std::move(E)) << '\n';
     return 1;
   }
   std::unique_ptr<ToolOutputFile> RemarksFile = std::move(*RemarksFileOrErr);
 
   // Load the input module...
-  auto SetDataLayout = [](StringRef) -> std::optional<std::string> {
+  auto SetDataLayout = [](const Module &) -> std::optional<std::string> {
     if (ClDataLayout.empty())
       return std::nullopt;
     return ClDataLayout;
   };
   std::unique_ptr<Module> M;
   if (NoUpgradeDebugInfo)
     M = parseAssemblyFileWithIndexNoUpgradeDebugInfo(
             InputFilename, Err, Context, nullptr, SetDataLayout)