[PATCH] MIR Serialization: Create dummy functions for the parsed machine functions when MIR file doesn't contain LLVM IR.

[Alex L]

Thanks, I committed the patch with your suggestions applied in r239778.

Alex.

2015-05-29 12:06 GMT-07:00 Duncan P. N. Exon Smith <dexonsmith at apple.com>:

>
> > On 2015-May-29, at 11:33, Alex Lorenz <arphaman at gmail.com> wrote:
> >
> > Hi dexonsmith, bogner, bob.wilson,
> >
> > This patch is based on the patch that connects MIRParser to the machine
> function analysis pass (http://reviews.llvm.org/D9928).
> >
> > This patch creates dummy LLVM functions with one basic block and a
> return instruction for the parsed machine functions when the MIR file
> doesn't include LLVM IR.
> > This is needed as the machine function analysis pass creates machine
> functions only for the functions defined in the current LLVM module.
> >
> > REPOSITORY
> >  rL LLVM
> >
> > http://reviews.llvm.org/D10135
> >
> > Files:
> >  lib/CodeGen/MIRParser/MIRParser.cpp
> >  test/CodeGen/MIR/llvmIRMissing.mir
> >
> I'm skeptical that testcases without backing LLVM IR are going to prove
> useful.  This particular patch doesn't seem to be too complicated, so
> I'm fine with it, but I think you should focus on serializing MIR that
> *has* matching LLVM IR.
>
> A few comments below; otherwise LGTM.
>
> > Index: lib/CodeGen/MIRParser/MIRParser.cpp
> > ===================================================================
> > --- lib/CodeGen/MIRParser/MIRParser.cpp
> > +++ lib/CodeGen/MIRParser/MIRParser.cpp
> > @@ -19,6 +19,7 @@
> >  #include "llvm/AsmParser/Parser.h"
> >  #include "llvm/CodeGen/MachineFunction.h"
> >  #include "llvm/CodeGen/MIRYamlMapping.h"
> > +#include "llvm/IR/Instructions.h"
> >  #include "llvm/IR/Module.h"
> >  #include "llvm/Support/SMLoc.h"
> >  #include "llvm/Support/SourceMgr.h"
> > @@ -51,13 +52,17 @@
> >    /// Parse the machine function in the current YAML document.
> >    ///
> >    /// Return true if an error occurred.
>
> You should document the effect that `NoLLVMIR` has.
>
> > -  bool parseMachineFunction(yaml::Input &In);
> > +  bool parseMachineFunction(yaml::Input &In, Module &M, bool NoLLVMIR);
> >
> >    /// 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:
> > +  /// Create an empty function with the given name.
> > +  void createDummyFunction(StringRef Name, Module &M);
> >  };
> >
> >  } // end namespace llvm
> > @@ -84,6 +89,7 @@
> >    }
> >
> >    std::unique_ptr<Module> M;
> > +  bool NoLLVMIR = false;
> >    // Parse the block scalar manually so that we can return unique
> pointer
> >    // without having to go trough YAML traits.
> >    if (const auto *BSN =
> > @@ -98,28 +104,40 @@
> >    } else {
> >      // Create an new, empty module.
> >      M = llvm::make_unique<Module>(Filename, Context);
> > +    NoLLVMIR = true;
> >    }
> >
> >    // Parse the machine functions.
> >    do {
> > -    if (parseMachineFunction(In))
> > +    if (parseMachineFunction(In, *M, NoLLVMIR))
> >        return nullptr;
> >      In.nextDocument();
> >    } while (In.setCurrentDocument());
> >
> >    return M;
> >  }
> >
> > -bool MIRParserImpl::parseMachineFunction(yaml::Input &In) {
> > +bool MIRParserImpl::parseMachineFunction(yaml::Input &In, Module &M,
> > +                                         bool NoLLVMIR) {
> >    auto MF = llvm::make_unique<yaml::MachineFunction>();
> >    yaml::yamlize(In, *MF, false);
> >    if (In.error())
> >      return true;
> >    auto FunctionName = MF->Name;
> >    Functions.insert(std::make_pair(FunctionName, std::move(MF)));
> > +  if (NoLLVMIR)
> > +    createDummyFunction(FunctionName, M);
> >    return false;
> >  }
> >
> > +void MIRParserImpl::createDummyFunction(StringRef Name, Module &M) {
> > +  auto &Context = M.getContext();
> > +  Function *F = cast<Function>(M.getOrInsertFunction(
> > +      Name, FunctionType::get(Type::getVoidTy(Context), false)));
> > +  BasicBlock *BB = BasicBlock::Create(Context, "Entry", F);
> > +  ReturnInst::Create(Context, nullptr, BB);
>
> `unreachable` is a simpler terminator than `ret void`, so I think it
> makes more sense in a dummy function.


> > +}
> > +
> >  bool MIRParserImpl::initializeMachineFunction(MachineFunction &MF,
> >                                                SMDiagnostic &Error) {
> >    auto It = Functions.find(MF.getName());
> > Index: test/CodeGen/MIR/llvmIRMissing.mir
> > ===================================================================
> > --- test/CodeGen/MIR/llvmIRMissing.mir
> > +++ test/CodeGen/MIR/llvmIRMissing.mir
> > @@ -1,5 +1,7 @@
> > -# RUN: llc -start-after branch-folder -stop-after branch-folder -o
> /dev/null %s
> > +# RUN: llc -start-after branch-folder -stop-after branch-folder -o
> /dev/null %s | FileCheck %s
> >  # This test ensures that the MIR parser accepts files without the LLVM
> IR.
> >
> >  ---
> > +# CHECK: name: foo
> > +name: foo
> >  ...
> >
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150615/03ee48ef/attachment.html>