[PATCH] MIR Serialization: Print and parse machine function names.
Alex L
arphaman at gmail.com
Thu May 21 16:32:40 PDT 2015
2015-05-21 15:42 GMT-07:00 Duncan P. N. Exon Smith <dexonsmith at apple.com>:
>
> > On 2015 May 21, at 15:14, Alex Lorenz <arphaman at gmail.com> wrote:
> >
> > I've rebased the patch and removed the unnecessary target details from
> test cases.
> >
> >
> > REPOSITORY
> > rL LLVM
> >
> > http://reviews.llvm.org/D9841
> >
> > Files:
> > lib/CodeGen/MIR/MIRParser.cpp
> > lib/CodeGen/MIR/MIRPrinter.cpp
> > lib/CodeGen/MIR/MIRPrinter.h
> > lib/CodeGen/MIR/MIRPrintingPass.cpp
> > lib/CodeGen/MIR/YAMLMapping.h
> > test/CodeGen/MIR/mf.mir
> > test/CodeGen/MIR/mfNameMissing.mir
>
> These test names are kind of opaque, and the capitalization will make
> it hard to tab-complete them on the command-line.
>
> I'd prefer:
>
> MIR/machine-function.mir
> MIR/invalid-machine-function-missing-name.mir
>
> or:
>
> MIR/machine-function.mir
> MIR/machine-function-missing-name.mir
>
>
> Anything along those lines is fine with me.
>
> >
> > EMAIL PREFERENCES
> > http://reviews.llvm.org/settings/panel/emailpreferences/
> > <D9841.26280.patch>
>
> > Index: lib/CodeGen/MIR/MIRParser.cpp
> > ===================================================================
> > --- lib/CodeGen/MIR/MIRParser.cpp
> > +++ lib/CodeGen/MIR/MIRParser.cpp
> > @@ -13,6 +13,7 @@
> >
> //===----------------------------------------------------------------------===//
> >
> > #include "llvm/CodeGen/MIR/MIRParser.h"
> > +#include "YAMLMapping.h"
> > #include "llvm/ADT/StringRef.h"
> > #include "llvm/ADT/STLExtras.h"
> > #include "llvm/AsmParser/Parser.h"
> > @@ -38,10 +39,16 @@
> > MIRParserImpl(std::unique_ptr<MemoryBuffer> Contents, StringRef
> Filename,
> > LLVMContext &Context);
> >
> > - /// Try to parse the optional LLVM module in the MIR file.
> > + /// Try to parse the optional LLVM module and the machine functions
> in the MIR
> > + /// file.
> > ///
> > - /// Return null if an error occurred while parsing the LLVM module.
> > - std::unique_ptr<Module> parseLLVMModule(SMDiagnostic &Error);
> > + /// Return null if an error occurred.
> > + std::unique_ptr<Module> parse(SMDiagnostic &Error);
> > +
> > + /// Parse the machine function in the current YAML document.
> > + ///
> > + /// Return true if an error occurred.
> > + bool parseMachineFunction(yaml::Input &In);
> > };
> >
> > } // end anonymous namespace
> > @@ -52,21 +59,53 @@
> > SM.AddNewSourceBuffer(std::move(Contents), SMLoc());
> > }
> >
> > -std::unique_ptr<Module> MIRParserImpl::parseLLVMModule(SMDiagnostic
> &Error) {
> > - yaml::Input In(SM.getMemoryBuffer(SM.getMainFileID())->getBuffer());
> > +static void handleYAMLDiag(const SMDiagnostic &Diag, void *Context) {
> > + *reinterpret_cast<SMDiagnostic *>(Context) = Diag;
> > +}
> >
> > - // Parse the block scalar manually so that we can return unique
> pointer
> > - // without having to go trough YAML traits.
> > - if (In.setCurrentDocument()) {
> > +std::unique_ptr<Module> MIRParserImpl::parse(SMDiagnostic &Error) {
> > + yaml::Input In(SM.getMemoryBuffer(SM.getMainFileID())->getBuffer(),
> > + /*Ctxt=*/nullptr, handleYAMLDiag, &Error);
>
> Can you clang-format this? The comment right against the `nullptr` is
> awkward.
>
Clang-format leaves it likes this, should I add a space between the comment
and nullptr?
>
> > +
> > + std::unique_ptr<Module> Mod;
>
> Nitpick: typically I've seen either full words or initializisms; since
> the full word is out, probably `M` makes sense here.
>
> > + bool DocumentStatus = In.setCurrentDocument();
> > + if (DocumentStatus) {
>
> Shouldn't this just return immediately if `false`? Maybe after an
> error message?
>
> > + // 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())) {
>
> Shouldn't this just return immediately if not `BSN` (maybe after an
> error message)? Or does the code that follows somehow subsume this?
>
No, this should continue even if the current node isn't BSN, since the LLVM
IR
is optional, so the first document doesn't have to be a BSN.
>
> > - return parseAssembly(MemoryBufferRef(BSN->getValue(), Filename),
> Error,
> > - Context);
> > + Mod = parseAssembly(MemoryBufferRef(BSN->getValue(), Filename),
> Error,
> > + Context);
> > + if (!Mod)
> > + return Mod;
> > + In.nextDocument();
> > }
> > }
> >
> > - // Create an new, empty module.
> > - return llvm::make_unique<Module>(Filename, Context);
> > + bool HasLLVMModule = (bool)Mod;
> > + if (!HasLLVMModule) // Create an new, empty module.
> > + Mod = llvm::make_unique<Module>(Filename, Context);
> > +
> > + // Parse the machine functions.
> > + if (HasLLVMModule)
> > + DocumentStatus = In.setCurrentDocument();
>
> This logic looks awkward. What about this?
>
> if (M)
> DocumentStatus = In.setCurrentDocument();
> else // Create an new, empty module.
> M = llvm::make_unique<Module>(Filename, Context);
>
> // Parse the machine functions.
>
> But couldn't this all get rolled up with the logic at the top?
>
> if (const auto *BSN = /* ... */) {
> M = parseAsembly(...);
> if (!M)
> return M;
> In.nextDocument();
> if (!In.setCurrentDocument())
> return M;
> } else {
> M = llvm::make_unique<Module>(...);
> }
>
> > + for (; DocumentStatus; DocumentStatus = In.setCurrentDocument()) {
>
> All this `DocumentStatus` control flow is pretty hard to follow. This
> one just looks like:
>
> do { /* ... */ } while (In.setCurrentDocument());
>
> > + if (parseMachineFunction(In))
> > + return nullptr;
> > + In.nextDocument();
> > + }
> > +
> > + return Mod;
> > +}
> > +
> > +bool MIRParserImpl::parseMachineFunction(yaml::Input &In) {
> > + yaml::MachineFunction MF;
> > + 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.
> > + return false;
> > }
> >
> > std::unique_ptr<Module> llvm::parseMIRFile(StringRef Filename,
> > @@ -86,5 +125,5 @@
> > LLVMContext &Context) {
> > auto Filename = Contents->getBufferIdentifier();
> > MIRParserImpl Parser(std::move(Contents), Filename, Context);
> > - return Parser.parseLLVMModule(Error);
> > + return Parser.parse(Error);
> > }
> > Index: lib/CodeGen/MIR/MIRPrinter.cpp
> > ===================================================================
> > --- lib/CodeGen/MIR/MIRPrinter.cpp
> > +++ lib/CodeGen/MIR/MIRPrinter.cpp
> > @@ -13,8 +13,10 @@
> >
> //===----------------------------------------------------------------------===//
> >
> > #include "MIRPrinter.h"
> > +#include "YAMLMapping.h"
> > #include "llvm/ADT/STLExtras.h"
> > #include "llvm/IR/Module.h"
> > +#include "llvm/CodeGen/MachineFunction.h"
> > #include "llvm/Support/MemoryBuffer.h"
> > #include "llvm/Support/raw_ostream.h"
> > #include "llvm/Support/YAMLTraits.h"
> > @@ -32,6 +34,8 @@
> > MIRPrinter(raw_ostream &OS);
> >
> > void printModule(const Module &Mod);
> > +
> > + void printFunction(const MachineFunction &MF);
> > };
> >
> > } // end anonymous namespace
> > @@ -60,7 +64,19 @@
> > Out << const_cast<Module &>(Mod);
> > }
> >
> > +void MIRPrinter::printFunction(const MachineFunction &MF) {
> > + yaml::MachineFunction YamlMF;
> > + YamlMF.Name = MF.getName();
> > + yaml::Output Out(OS);
> > + Out << YamlMF;
> > +}
> > +
> > void llvm::printMIR(raw_ostream &OS, const Module &Mod) {
> > MIRPrinter Printer(OS);
> > Printer.printModule(Mod);
> > }
> > +
> > +void llvm::printMIR(raw_ostream &OS, const MachineFunction &MF) {
> > + MIRPrinter Printer(OS);
> > + Printer.printFunction(MF);
> > +}
> > Index: lib/CodeGen/MIR/MIRPrinter.h
> > ===================================================================
> > --- lib/CodeGen/MIR/MIRPrinter.h
> > +++ lib/CodeGen/MIR/MIRPrinter.h
> > @@ -7,23 +7,27 @@
> > //
> >
> //===----------------------------------------------------------------------===//
> > //
> > -// This file declares the function that prints out the LLVM IR using
> the MIR
> > -// serialization format.
> > -// TODO: Print out machine functions.
> > +// This file declares the functions that print out the LLVM IR and
> machine
> > +// functions using the MIR serialization format.
> > //
> >
> //===----------------------------------------------------------------------===//
> >
> > #ifndef LLVM_LIB_CODEGEN_MIR_MIRPRINTER_H
> > #define LLVM_LIB_CODEGEN_MIR_MIRPRINTER_H
> >
> > namespace llvm {
> >
> > +class MachineFunction;
> > class Module;
> > class raw_ostream;
> >
> > /// Print LLVM IR using the MIR serialization format to the given
> output stream.
> > void printMIR(raw_ostream &OS, const Module &Mod);
> >
> > +/// Print a machine function using the MIR serialization format to the
> given
> > +/// output stream.
> > +void printMIR(raw_ostream &OS, const MachineFunction &MF);
> > +
> > } // end namespace llvm
> >
> > #endif
> > Index: lib/CodeGen/MIR/MIRPrintingPass.cpp
> > ===================================================================
> > --- lib/CodeGen/MIR/MIRPrintingPass.cpp
> > +++ lib/CodeGen/MIR/MIRPrintingPass.cpp
> > @@ -28,6 +28,7 @@
> > struct MIRPrintingPass : public MachineFunctionPass {
> > static char ID;
> > raw_ostream &OS;
> > + std::string MachineFunctions;
> >
> > MIRPrintingPass() : MachineFunctionPass(ID), OS(dbgs()) {}
> > MIRPrintingPass(raw_ostream &OS) : MachineFunctionPass(ID), OS(OS) {}
> > @@ -40,12 +41,16 @@
> > }
> >
> > virtual bool runOnMachineFunction(MachineFunction &MF) override {
> > - // TODO: Print out the machine function.
> > + std::string Str;
> > + raw_string_ostream StrOS(Str);
> > + printMIR(StrOS, MF);
> > + MachineFunctions.append(StrOS.str());
> > return false;
> > }
> >
> > virtual bool doFinalization(Module &M) override {
> > printMIR(OS, M);
> > + OS << MachineFunctions;
> > return false;
> > }
> > };
> > Index: lib/CodeGen/MIR/YAMLMapping.h
> > ===================================================================
> > --- /dev/null
> > +++ lib/CodeGen/MIR/YAMLMapping.h
> > @@ -0,0 +1,37 @@
> > +//===- YAMLMapping.h - Describes the mapping between MIR and YAML
> ---------===//
> > +//
> > +// The LLVM Compiler Infrastructure
> > +//
> > +// This file is distributed under the University of Illinois Open Source
> > +// License. See LICENSE.TXT for details.
> > +//
> >
> +//===----------------------------------------------------------------------===//
> > +//
> > +// This file implements the mapping between various MIR data structures
> and
> > +// their corresponding YAML representation.
> > +//
> >
> +//===----------------------------------------------------------------------===//
> > +
> > +#ifndef LLVM_LIB_CODEGEN_MIR_YAMLMAPPING_H
> > +#define LLVM_LIB_CODEGEN_MIR_YAMLMAPPING_H
> > +
> > +#include "llvm/ADT/StringRef.h"
> > +#include "llvm/Support/YAMLTraits.h"
> > +
> > +namespace llvm {
> > +namespace yaml {
> > +
> > +struct MachineFunction {
> > + StringRef Name;
> > +};
> > +
> > +template <> struct MappingTraits<MachineFunction> {
> > + static void mapping(IO &YamlIO, MachineFunction &MF) {
> > + YamlIO.mapRequired("name", MF.Name);
> > + }
> > +};
> > +
> > +} // end namespace yaml
> > +} // end namespace llvm
> > +
> > +#endif
> > Index: test/CodeGen/MIR/mf.mir
> > ===================================================================
> > --- /dev/null
> > +++ test/CodeGen/MIR/mf.mir
> > @@ -0,0 +1,24 @@
> > +# RUN: llc -start-after branch-folder -stop-after branch-folder -o
> /dev/null %s | FileCheck %s
> > +# This test ensures that the MIR parser parses machine functions
> correctly.
> > +
> > +--- |
> > +
> > + define i32 @foo() {
> > + ret i32 0
> > + }
> > +
> > + define i32 @bar() {
> > + ret i32 0
> > + }
> > +
> > +...
> > +---
> > +# CHECK: name: foo
> > +# CHECK-NEXT: ...
> > +name: foo
> > +...
> > +---
> > +# CHECK: name: bar
> > +# CHECK-NEXT: ...
> > +name: bar
> > +...
> > Index: test/CodeGen/MIR/mfNameMissing.mir
> > ===================================================================
> > --- /dev/null
> > +++ test/CodeGen/MIR/mfNameMissing.mir
> > @@ -0,0 +1,22 @@
> > +# RUN: not llc -start-after branch-folder -stop-after branch-folder -o
> /dev/null %s 2>&1 | FileCheck %s
> > +# This test ensures that an error is reported when a machine function
> doesn't
> > +# have a name attribute.
> > +
> > +--- |
> > +
> > + define i32 @foo() {
> > + ret i32 0
> > + }
> > +
> > + define i32 @bar() {
> > + ret i32 0
> > + }
> > +
> > +...
> > +---
> > +# CHECK: [[@LINE+1]]:1: error: missing required key 'name'
> > +nme: foo
> > +...
> > +---
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150521/11061e2e/attachment.html>
More information about the llvm-commits
mailing list