[PATCH] MIR Serialization: Connect the machine function analysis pass to MIR parser.

Alex L arphaman at gmail.com
Tue Jun 9 16:31:38 PDT 2015


2015-06-09 13:30 GMT-07:00 Justin Bogner <mail at justinbogner.com>:

> Alex Lorenz <arphaman at gmail.com> writes:
> > Rebase on master.
> >
> > REPOSITORY
> >   rL LLVM
> >
> > http://reviews.llvm.org/D9928
> >
> > Files:
> >   include/llvm/CodeGen/MIRMachineFunctionInitializer.h
> >   include/llvm/CodeGen/MIRParser/MIRParser.h
> >   include/llvm/CodeGen/MachineFunctionAnalysis.h
> >   include/llvm/Target/TargetMachine.h
> >   lib/CodeGen/LLVMTargetMachine.cpp
> >   lib/CodeGen/MIRParser/MIRParser.cpp
> >   lib/CodeGen/MachineFunctionAnalysis.cpp
> >   lib/Target/CppBackend/CPPBackend.cpp
> >   lib/Target/CppBackend/CPPTargetMachine.h
> >   test/CodeGen/MIR/function-missing-machine-function.mir
> >   test/CodeGen/MIR/llvmIR.mir
> >   tools/llc/llc.cpp
> >
> > EMAIL PREFERENCES
> >   http://reviews.llvm.org/settings/panel/emailpreferences/
> >
> > Index: include/llvm/CodeGen/MIRMachineFunctionInitializer.h
> > ===================================================================
> > --- /dev/null
> > +++ include/llvm/CodeGen/MIRMachineFunctionInitializer.h
> > @@ -0,0 +1,40 @@
> > +//===- MIRMachineFunctionInitalizer.h - MIR machine function
> initializer  -===//
> > +//
> > +//                     The LLVM Compiler Infrastructure
> > +//
> > +// This file is distributed under the University of Illinois Open Source
> > +// License. See LICENSE.TXT for details.
> > +//
> >
> +//===----------------------------------------------------------------------===//
> > +//
> > +// This MIR serialization library is currently a work in progress. It
> can't
> > +// serialize machine functions at this time.
> > +//
> > +// This file declares an interface that initializes the machine
> functions by
> > +// applying the state loaded from a MIR file.
> > +//
> >
> +//===----------------------------------------------------------------------===//
> > +
> > +#ifndef LLVM_CODEGEN_MIRMACHINEFUNCTIONINITIALIZER_H
> > +#define LLVM_CODEGEN_MIRMACHINEFUNCTIONINITIALIZER_H
> >
> > +
> > +namespace llvm {
> > +
> > +class MachineFunction;
> > +
> > +/// This interface initializes machine functions by applying the state
> loaded
> > +/// from a MIR file.
> > +class MIRMachineFunctionInitializer {
> >
> > +public:
> > +  virtual ~MIRMachineFunctionInitializer() {}
> > +
> > +  /// Initialize the machine function to the state that's described in
> the MIR
> > +  /// file.
> > +  ///
> > +  /// Return true if error occurred.
> > +  virtual bool initializeMachineFunction(MachineFunction &MF) = 0;
>
> If this is all that's going to end up in this interface, it really
> doesn't have anything to do with MIR, right? Maybe it should just be
> called MachineFunctionInitializer?
>
> On that note, most of the comments here are written as if they're for
> MIRParser - this class really knows nothing about MIR or files or
> anything.
>
> > +};
> > +
> > +} // end namespace llvm
> > +
> > +#endif
> > Index: include/llvm/CodeGen/MIRParser/MIRParser.h
> > ===================================================================
> > --- include/llvm/CodeGen/MIRParser/MIRParser.h
> > +++ include/llvm/CodeGen/MIRParser/MIRParser.h
> > @@ -19,33 +19,63 @@
> >  #define LLVM_CODEGEN_MIRPARSER_MIRPARSER_H
> >
> >  #include "llvm/ADT/StringRef.h"
> > +#include "llvm/CodeGen/MIRMachineFunctionInitializer.h"
> >  #include "llvm/IR/Module.h"
> >  #include "llvm/Support/MemoryBuffer.h"
> > +#include "llvm/Support/SourceMgr.h"
> >  #include <memory>
> >
> >  namespace llvm {
> >
> > -class SMDiagnostic;
> > +class MachineFunction;
> > +class MIRParserImpl;
> > +
> > +/// This class initializes machine functions by applying the state
> loaded from
> > +/// a MIR file.
> > +class MIRParser : public MIRMachineFunctionInitializer {
> > +  std::unique_ptr<MIRParserImpl> Impl;
> > +  SMDiagnostic Error;
> > +
> > +public:
> > +  MIRParser(std::unique_ptr<MIRParserImpl> Impl);
> > +  MIRParser(const MIRParser &) = delete;
> > +  ~MIRParser();
> > +
> > +  /// Initialize the machine function to the state that's described in
> the MIR
> > +  /// file.
> > +  ///
> > +  /// Return true if error occurred.
> > +  bool initializeMachineFunction(MachineFunction &MF) override;
> > +
> > +  /// Return true if the machine function initialization failed during
> the
> > +  /// machine function analysis pass.
> > +  bool failed() const { return !Error.getMessage().empty(); }
> > +
> > +  const SMDiagnostic &error() const { return Error; }
>
> It depends a bit on exactly what kind of errors we're expecting out of
> this, but I suspect this isn't the best model for error handling in this
> class. As is, you'll only be able to report exactly one issue, and it
> can't be fatal.
>
> Have you looked at DiagnosticInfo/LLVMContext::diagnose at all?  As long
> as your errors are fairly structured it might give you more flexibility.
>

Thanks, I wasn't aware of DiagnosticInfo. My updated patch now uses it.

It's not really a perfect fit, as I still use SMDiagnostics with it but it
works better
than the previous system as llc can now exit on the first error, and other
users
of the MIRParser library can get multiple errors if they want.


>
> Otherwise, if you think the single SMDiagnostic model will work well for
> you going forward, please call these hasError and getError, for
> consistency with other interfaces.


> > +};
> > +
> > +typedef std::pair<std::unique_ptr<Module>, std::unique_ptr<MIRParser>>
> > +    MIRParseResult;
> >
> >  /// This function is the main interface to the MIR serialization format
> parser.
> >  ///
> >  /// It reads a YAML file that has an optional LLVM IR and returns an
> LLVM
> >  /// module.
> >  /// \param Filename - The name of the file to parse.
> >  /// \param Error - Error result info.
> >  /// \param Context - Context in which to allocate globals info.
> > -std::unique_ptr<Module> parseMIRFile(StringRef Filename, SMDiagnostic
> &Error,
> > -                                     LLVMContext &Context);
> > +MIRParseResult parseMIRFile(StringRef Filename, SMDiagnostic &Error,
> > +                            LLVMContext &Context);
> >
> >  /// This function is another interface to the MIR serialization format
> parser.
> >  ///
> >  /// It parses the optional LLVM IR in the given buffer, and returns an
> LLVM
> >  /// module.
> >  /// \param Contents - The MemoryBuffer containing the machine level IR.
> >  /// \param Error - Error result info.
> >  /// \param Context - Context in which to allocate globals info.
> > -std::unique_ptr<Module> parseMIR(std::unique_ptr<MemoryBuffer> Contents,
> > -                                 SMDiagnostic &Error, LLVMContext
> &Context);
> > +MIRParseResult parseMIR(std::unique_ptr<MemoryBuffer> Contents,
> > +                        SMDiagnostic &Error, LLVMContext &Context);
> >
> >
> >  } // end namespace llvm
> >
> > Index: include/llvm/CodeGen/MachineFunctionAnalysis.h
> > ===================================================================
> > --- include/llvm/CodeGen/MachineFunctionAnalysis.h
> > +++ include/llvm/CodeGen/MachineFunctionAnalysis.h
> > @@ -19,6 +19,7 @@
> >  namespace llvm {
> >
> >  class MachineFunction;
> > +class MIRMachineFunctionInitializer;
> >  class TargetMachine;
> >
> >  /// MachineFunctionAnalysis - This class is a Pass that manages a
> > @@ -28,9 +29,12 @@
> >    const TargetMachine &TM;
> >    MachineFunction *MF;
> >    unsigned NextFnNum;
> > +  MIRMachineFunctionInitializer *MFInitializer;
> > +
> >  public:
> >    static char ID;
> > -  explicit MachineFunctionAnalysis(const TargetMachine &tm);
> > +  explicit MachineFunctionAnalysis(
> > +      const TargetMachine &tm, MIRMachineFunctionInitializer
> *MFInitializer);
> >    ~MachineFunctionAnalysis() override;
> >
> >    MachineFunction &getMF() const { return *MF; }
> > Index: include/llvm/Target/TargetMachine.h
> > ===================================================================
> > --- include/llvm/Target/TargetMachine.h
> > +++ include/llvm/Target/TargetMachine.h
> > @@ -27,6 +27,7 @@
> >  class InstrItineraryData;
> >  class GlobalValue;
> >  class Mangler;
> > +class MIRMachineFunctionInitializer;
> >  class MCAsmInfo;
> >  class MCCodeGenInfo;
> >  class MCContext;
> > @@ -208,11 +209,11 @@
> >    /// emitted.  Typically this will involve several steps of code
> generation.
> >    /// This method should return true if emission of this file type is
> not
> >    /// supported, or false on success.
> > -  virtual bool addPassesToEmitFile(PassManagerBase &, raw_pwrite_stream
> &,
> > -                                   CodeGenFileType,
> > -                                   bool /*DisableVerify*/ = true,
> > -                                   AnalysisID /*StartAfter*/ = nullptr,
> > -                                   AnalysisID /*StopAfter*/ = nullptr) {
> > +  virtual bool addPassesToEmitFile(
> > +      PassManagerBase &, raw_pwrite_stream &, CodeGenFileType,
> > +      bool /*DisableVerify*/ = true, AnalysisID /*StartAfter*/ =
> nullptr,
> > +      AnalysisID /*StopAfter*/ = nullptr,
> > +      MIRMachineFunctionInitializer * /*MFInitializer*/ = nullptr) {
> >      return true;
> >    }
> >
> > @@ -256,10 +257,11 @@
> >
> >    /// Add passes to the specified pass manager to get the specified file
> >    /// emitted.  Typically this will involve several steps of code
> generation.
> > -  bool addPassesToEmitFile(PassManagerBase &PM, raw_pwrite_stream &Out,
> > -                           CodeGenFileType FileType, bool DisableVerify
> = true,
> > -                           AnalysisID StartAfter = nullptr,
> > -                           AnalysisID StopAfter = nullptr) override;
> > +  bool addPassesToEmitFile(
> > +      PassManagerBase &PM, raw_pwrite_stream &Out, CodeGenFileType
> FileType,
> > +      bool DisableVerify = true, AnalysisID StartAfter = nullptr,
> > +      AnalysisID StopAfter = nullptr,
> > +      MIRMachineFunctionInitializer *MFInitializer = nullptr) override;
> >
> >    /// Add passes to the specified pass manager to get machine code
> emitted with
> >    /// the MCJIT. This method returns true if machine code is not
> supported. It
> > Index: lib/CodeGen/LLVMTargetMachine.cpp
> > ===================================================================
> > --- lib/CodeGen/LLVMTargetMachine.cpp
> > +++ lib/CodeGen/LLVMTargetMachine.cpp
> > @@ -87,11 +87,10 @@
> >  }
> >
> >  /// addPassesToX helper drives creation and initialization of
> TargetPassConfig.
> > -static MCContext *addPassesToGenerateCode(LLVMTargetMachine *TM,
> > -                                          PassManagerBase &PM,
> > -                                          bool DisableVerify,
> > -                                          AnalysisID StartAfter,
> > -                                          AnalysisID StopAfter) {
> > +static MCContext *addPassesToGenerateCode(
> > +    LLVMTargetMachine *TM, PassManagerBase &PM, bool DisableVerify,
> > +    AnalysisID StartAfter, AnalysisID StopAfter,
> > +    MIRMachineFunctionInitializer *MFInitializer = nullptr) {
> >
> >    // Add internal analysis passes from the target machine.
> >
> PM.add(createTargetTransformInfoWrapperPass(TM->getTargetIRAnalysis()));
> > @@ -121,7 +120,7 @@
> >    PM.add(MMI);
> >
> >    // Set up a MachineFunction for the rest of CodeGen to work on.
> > -  PM.add(new MachineFunctionAnalysis(*TM));
> > +  PM.add(new MachineFunctionAnalysis(*TM, MFInitializer));
> >
> >    // Enable FastISel with -fast, but allow that to be overridden.
> >    if (EnableFastISelOption == cl::BOU_TRUE ||
> > @@ -142,10 +141,11 @@
> >
> >  bool LLVMTargetMachine::addPassesToEmitFile(
> >      PassManagerBase &PM, raw_pwrite_stream &Out, CodeGenFileType
> FileType,
> > -    bool DisableVerify, AnalysisID StartAfter, AnalysisID StopAfter) {
> > +    bool DisableVerify, AnalysisID StartAfter, AnalysisID StopAfter,
> > +    MIRMachineFunctionInitializer *MFInitializer) {
> >    // Add common CodeGen passes.
> > -  MCContext *Context = addPassesToGenerateCode(this, PM, DisableVerify,
> > -                                               StartAfter, StopAfter);
> > +  MCContext *Context = addPassesToGenerateCode(
> > +      this, PM, DisableVerify, StartAfter, StopAfter, MFInitializer);
> >    if (!Context)
> >      return true;
> >
> > Index: lib/CodeGen/MIRParser/MIRParser.cpp
> > ===================================================================
> > --- lib/CodeGen/MIRParser/MIRParser.cpp
> > +++ lib/CodeGen/MIRParser/MIRParser.cpp
> > @@ -14,8 +14,10 @@
> >
> >  #include "llvm/CodeGen/MIRParser/MIRParser.h"
> >  #include "llvm/ADT/StringRef.h"
> > +#include "llvm/ADT/StringMap.h"
> >  #include "llvm/ADT/STLExtras.h"
> >  #include "llvm/AsmParser/Parser.h"
> > +#include "llvm/CodeGen/MachineFunction.h"
> >  #include "llvm/CodeGen/MIRYamlMapping.h"
> >  #include "llvm/IR/Module.h"
> >  #include "llvm/Support/LineIterator.h"
> > @@ -27,14 +29,15 @@
> >
> >  using namespace llvm;
> >
> > -namespace {
> > +namespace llvm {
> >
> >  /// This class implements the parsing of LLVM IR that's embedded inside
> a MIR
> >  /// file.
> >  class MIRParserImpl {
> >    SourceMgr SM;
> >    StringRef Filename;
> >    LLVMContext &Context;
> > +  StringMap<std::unique_ptr<yaml::MachineFunction>> Functions;
> >
> >  public:
> >    MIRParserImpl(std::unique_ptr<MemoryBuffer> Contents, StringRef
> Filename,
> > @@ -51,13 +54,19 @@
> >    /// Return true if an error occurred.
> >    bool parseMachineFunction(yaml::Input &In);
> >
> > +  /// Initialize the machine function to the state that's described in
> the MIR
> > +  /// file.
> > +  ///
> > +  /// Return true if error occurred.
> > +  bool initializeMachineFunction(MachineFunction &MF, SMDiagnostic
> &Error);
> > +
> >  private:
> >    /// Return a MIR diagnostic converted from an LLVM assembly
> diagnostic.
> >    SMDiagnostic diagFromLLVMAssemblyDiag(const SMDiagnostic &Error,
> >                                          SMRange SourceRange);
> >  };
> >
> > -} // end anonymous namespace
> > +} // end namespace llvm
> >
> >  MIRParserImpl::MIRParserImpl(std::unique_ptr<MemoryBuffer> Contents,
> >                               StringRef Filename, LLVMContext &Context)
> > @@ -110,12 +119,26 @@
> >  }
> >
> >  bool MIRParserImpl::parseMachineFunction(yaml::Input &In) {
> > -  yaml::MachineFunction MF;
> > -  yaml::yamlize(In, MF, false);
> > +  auto MF = llvm::make_unique<yaml::MachineFunction>();
> > +  yaml::yamlize(In, *MF, false);
> >    if (In.error())
> >      return true;
> > -  // TODO: Initialize the real machine function with the state in the
> yaml
> > -  // machine function later on.
> > +  auto FunctionName = MF->Name;
> > +  Functions.insert(std::make_pair(FunctionName, std::move(MF)));
> > +  return false;
> > +}
> > +
> > +bool MIRParserImpl::initializeMachineFunction(MachineFunction &MF,
> > +                                              SMDiagnostic &Error) {
> > +  auto It = Functions.find(MF.getName());
> > +  if (It == Functions.end()) {
> > +    auto Message = (Twine("No machine function information for function
> '") +
> > +                    MF.getName() + "' in the MIR file")
> > +                       .str();
> > +    Error = SMDiagnostic(Filename, SourceMgr::DK_Error, Message);
> > +    return true;
> > +  }
> > +  // TODO: Recreate the machine function.
> >    return false;
> >  }
> >
> > @@ -150,22 +173,34 @@
> >                        Error.getFixIts());
> >  }
> >
> > -std::unique_ptr<Module> llvm::parseMIRFile(StringRef Filename,
> > -                                           SMDiagnostic &Error,
> > -                                           LLVMContext &Context) {
> > +MIRParser::MIRParser(std::unique_ptr<MIRParserImpl> Impl)
> > +    : Impl(std::move(Impl)) {}
> > +
> > +MIRParser::~MIRParser() {}
> > +
> > +bool MIRParser::initializeMachineFunction(MachineFunction &MF) {
> > +  return Impl->initializeMachineFunction(MF, Error);
> > +}
> > +
> > +MIRParseResult llvm::parseMIRFile(StringRef Filename, SMDiagnostic
> &Error,
> > +                                  LLVMContext &Context) {
> >    auto FileOrErr = MemoryBuffer::getFile(Filename);
> >    if (std::error_code EC = FileOrErr.getError()) {
> >      Error = SMDiagnostic(Filename, SourceMgr::DK_Error,
> >                           "Could not open input file: " + EC.message());
> > -    return std::unique_ptr<Module>();
> > +    return MIRParseResult();
> >    }
> >    return parseMIR(std::move(FileOrErr.get()), Error, Context);
> >  }
> >
> > -std::unique_ptr<Module> llvm::parseMIR(std::unique_ptr<MemoryBuffer>
> Contents,
> > -                                       SMDiagnostic &Error,
> > -                                       LLVMContext &Context) {
> > +MIRParseResult llvm::parseMIR(std::unique_ptr<MemoryBuffer> Contents,
> > +                              SMDiagnostic &Error, LLVMContext
> &Context) {
> >    auto Filename = Contents->getBufferIdentifier();
> > -  MIRParserImpl Parser(std::move(Contents), Filename, Context);
> > -  return Parser.parse(Error);
> > +  auto Parser =
> > +      llvm::make_unique<MIRParserImpl>(std::move(Contents), Filename,
> Context);
> > +  auto Mod = Parser->parse(Error);
> > +  if (!Mod)
> > +    return MIRParseResult();
>
> I find MIRParseResult and how we're returning this pretty awkward. Note
> here that both parseMIRFile and parseMIR create the parser first, then
> just call parse. More below.
>
> > +  return std::make_pair(std::move(Mod),
> > +
> llvm::make_unique<MIRParser>(std::move(Parser)));
> >  }
> > Index: lib/CodeGen/MachineFunctionAnalysis.cpp
> > ===================================================================
> > --- lib/CodeGen/MachineFunctionAnalysis.cpp
> > +++ lib/CodeGen/MachineFunctionAnalysis.cpp
> > @@ -15,12 +15,14 @@
> >  #include "llvm/CodeGen/GCMetadata.h"
> >  #include "llvm/CodeGen/MachineFunction.h"
> >  #include "llvm/CodeGen/MachineModuleInfo.h"
> > +#include "llvm/CodeGen/MIRMachineFunctionInitializer.h"
> >  using namespace llvm;
> >
> >  char MachineFunctionAnalysis::ID = 0;
> >
> > -MachineFunctionAnalysis::MachineFunctionAnalysis(const TargetMachine
> &tm) :
> > -  FunctionPass(ID), TM(tm), MF(nullptr) {
> > +MachineFunctionAnalysis::MachineFunctionAnalysis(
> > +    const TargetMachine &tm, MIRMachineFunctionInitializer
> *MFInitializer)
> > +    : FunctionPass(ID), TM(tm), MF(nullptr),
> MFInitializer(MFInitializer) {
> >    initializeMachineModuleInfoPass(*PassRegistry::getPassRegistry());
> >  }
> >
> > @@ -47,6 +49,8 @@
> >    assert(!MF && "MachineFunctionAnalysis already initialized!");
> >    MF = new MachineFunction(&F, TM, NextFnNum++,
> >                             getAnalysis<MachineModuleInfo>());
> > +  if (MFInitializer)
> > +    MFInitializer->initializeMachineFunction(*MF);
> >    return false;
> >  }
> >
> > Index: lib/Target/CppBackend/CPPBackend.cpp
> > ===================================================================
> > --- lib/Target/CppBackend/CPPBackend.cpp
> > +++ lib/Target/CppBackend/CPPBackend.cpp
> > @@ -2148,7 +2148,8 @@
> >
> >  bool CPPTargetMachine::addPassesToEmitFile(
> >      PassManagerBase &PM, raw_pwrite_stream &o, CodeGenFileType FileType,
> > -    bool DisableVerify, AnalysisID StartAfter, AnalysisID StopAfter) {
> > +    bool DisableVerify, AnalysisID StartAfter, AnalysisID StopAfter,
> > +    MIRMachineFunctionInitializer *MFInitializer) {
> >    if (FileType != TargetMachine::CGFT_AssemblyFile)
> >      return true;
> >    auto FOut = llvm::make_unique<formatted_raw_ostream>(o);
> > Index: lib/Target/CppBackend/CPPTargetMachine.h
> > ===================================================================
> > --- lib/Target/CppBackend/CPPTargetMachine.h
> > +++ lib/Target/CppBackend/CPPTargetMachine.h
> > @@ -29,10 +29,11 @@
> >        : TargetMachine(T, "", TT, CPU, FS, Options) {}
> >
> >  public:
> > -  bool addPassesToEmitFile(PassManagerBase &PM, raw_pwrite_stream &Out,
> > -                           CodeGenFileType FileType, bool DisableVerify,
> > -                           AnalysisID StartAfter,
> > -                           AnalysisID StopAfter) override;
> > +  bool
> > +  addPassesToEmitFile(PassManagerBase &PM, raw_pwrite_stream &Out,
> > +                      CodeGenFileType FileType, bool DisableVerify,
> > +                      AnalysisID StartAfter, AnalysisID StopAfter,
> > +                      MIRMachineFunctionInitializer *MFInitializer)
> override;
> >  };
> >
> >  extern Target TheCppBackendTarget;
> > Index: test/CodeGen/MIR/function-missing-machine-function.mir
> > ===================================================================
> > --- /dev/null
> > +++ test/CodeGen/MIR/function-missing-machine-function.mir
> > @@ -0,0 +1,13 @@
> > +# RUN: not llc -start-after branch-folder -stop-after branch-folder -o
> /dev/null %s 2>&1 | FileCheck %s
> > +# This test verifies that an error is reported when a MIR file has some
> > +# function but is missing a corresponding machine function.
> > +
> > +# CHECK: No machine function information for function 'foo' in the MIR
> file
> > +
> > +--- |
> > +
> > +  define i32 @foo() {
> > +    ret i32 0
> > +  }
> > +
> > +...
> > Index: test/CodeGen/MIR/llvmIR.mir
> > ===================================================================
> > --- test/CodeGen/MIR/llvmIR.mir
> > +++ test/CodeGen/MIR/llvmIR.mir
> > @@ -30,3 +30,6 @@
> >    }
> >
> >  ...
> > +---
> > +name: foo
> > +...
> > Index: tools/llc/llc.cpp
> > ===================================================================
> > --- tools/llc/llc.cpp
> > +++ tools/llc/llc.cpp
> > @@ -210,16 +210,19 @@
> >    // Load the module to be compiled...
> >    SMDiagnostic Err;
> >    std::unique_ptr<Module> M;
> > +  std::unique_ptr<MIRParser> MIR;
> >    Triple TheTriple;
> >
> >    bool SkipModule = MCPU == "help" ||
> >                      (!MAttrs.empty() && MAttrs.front() == "help");
> >
> >    // If user just wants to list available options, skip module loading
> >    if (!SkipModule) {
> > -    if (StringRef(InputFilename).endswith_lower(".mir"))
> > -      M = parseMIRFile(InputFilename, Err, Context);
> > -    else
> > +    if (StringRef(InputFilename).endswith_lower(".mir")) {
> > +      auto ParseResult = parseMIRFile(InputFilename, Err, Context);
> > +      M = std::move(ParseResult.first);
> > +      MIR = std::move(ParseResult.second);
>
> So it's pretty confusing here as a user of parseMIRFile. It would be
> simpler to create the MIRParser and parse the module as separate
> steps. Something along the lines of:
>
>   MIR = createMIRParserFromFile(InputFilename, Err, Context);
>   if (MIR)
>     M = MIR->parse()
>
> This avoids having to dig into .first and .second, is pretty much the
> same amount of code, and would make it easier if we needed to change
> from unique_ptr to ErrorOr or something in the future.
>
> > +    } else
> >        M = parseIRFile(InputFilename, Err, Context);
> >      if (!M) {
> >        Err.print(argv[0], errs());
> > @@ -350,7 +353,7 @@
> >
> >      // Ask the target to add backend passes as necessary.
> >      if (Target->addPassesToEmitFile(PM, *OS, FileType, NoVerify,
> StartAfterID,
> > -                                    StopAfterID)) {
> > +                                    StopAfterID, MIR.get())) {
> >        errs() << argv[0] << ": target does not support generation of
> this"
> >               << " file type!\n";
> >        return 1;
> > @@ -362,6 +365,13 @@
> >      PM.run(*M);
> >    }
> >
> > +  // Check if machine function loading failed in the machine function
> analysis
> > +  // pass.
> > +  if (MIR && MIR->failed()) {
> > +    MIR->error().print(argv[0], errs());
> > +    return 1;
> > +  }
> > +
> >    // Declare success.
> >    Out->keep();
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150609/9197d198/attachment.html>


More information about the llvm-commits mailing list