<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">2015-06-11 15:57 GMT-07:00 Justin Bogner <span dir="ltr"><<a href="mailto:mail@justinbogner.com" target="_blank">mail@justinbogner.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">Alex Lorenz <<a href="mailto:arphaman@gmail.com">arphaman@gmail.com</a>> writes:<br>
> I've updated the patch based on Justin's comments:<br>
><br>
> - renamed MIRMachineFunctionInitializer to MachineFunctionInitializer<br>
> - simplified the MIRParser library interface - now the functions only<br>
> create the parser, they don't parse the LLVM module. The LLVM module<br>
> can be parsed by calling the appropriate method from the MIRParser<br>
> class.<br>
<br>
</span>Thanks.<br>
<span class=""><br>
> - reworked the error handling to use LLVM's diagnostic info.<br>
<br>
</span>The particular mix of SMDiagnostic and DiagnosticInfo we've ended up<br>
with here is kind of confusing and awkward. We should probably take the<br>
SMDiagnostic out of the MachineFunctionInitializer::parse API entirely<br>
and use LLVMContext::diagnose consistently:<br></blockquote><div><br></div><div>That's a good idea, but I still would like keep the SMDiagnostic inside of DiagnosticInfo (I explain why below).</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
><br>
> I also added the 'anchor' method to the MachineFunctionInitializer interface.<br>
><br>
><br>
> REPOSITORY<br>
> rL LLVM<br>
><br>
> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D9928&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=tjZc8XHT_3Kj5WJ5rl6zEZA5EZNfvyt2tkW1yZPftSk&s=7dlvYr3FaaSluTFHhGmqlR6IdS009ANIZ2uFDgsO-e0&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/D9928</a><br>
><br>
> Files:<br>
> include/llvm/CodeGen/MIRParser/MIRParser.h<br>
> include/llvm/CodeGen/MachineFunctionAnalysis.h<br>
> include/llvm/CodeGen/MachineFunctionInitializer.h<br>
> include/llvm/IR/DiagnosticInfo.h<br>
> include/llvm/Target/TargetMachine.h<br>
> lib/CodeGen/LLVMTargetMachine.cpp<br>
> lib/CodeGen/MIRParser/MIRParser.cpp<br>
> lib/CodeGen/MachineFunction.cpp<br>
> lib/CodeGen/MachineFunctionAnalysis.cpp<br>
> lib/IR/DiagnosticInfo.cpp<br>
> lib/Target/CppBackend/CPPBackend.cpp<br>
> lib/Target/CppBackend/CPPTargetMachine.h<br>
> test/CodeGen/MIR/function-missing-machine-function.mir<br>
> test/CodeGen/MIR/llvmIR.mir<br>
> tools/llc/llc.cpp<br>
><br>
> EMAIL PREFERENCES<br>
> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_settings_panel_emailpreferences_&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=tjZc8XHT_3Kj5WJ5rl6zEZA5EZNfvyt2tkW1yZPftSk&s=bP3Ep3jVVQq4dEoUnEYGjp_A4HLaSVaFyKIZq1hjTJo&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
><br>
</span><span class="">> Index: include/llvm/CodeGen/MIRParser/MIRParser.h<br>
> ===================================================================<br>
> --- include/llvm/CodeGen/MIRParser/MIRParser.h<br>
> +++ include/llvm/CodeGen/MIRParser/MIRParser.h<br>
</span>> @@ -19,33 +19,62 @@<br>
> #define LLVM_CODEGEN_MIRPARSER_MIRPARSER_H<br>
><br>
> #include "llvm/ADT/StringRef.h"<br>
> +#include "llvm/CodeGen/MachineFunctionInitializer.h"<br>
> #include "llvm/IR/Module.h"<br>
> #include "llvm/Support/MemoryBuffer.h"<br>
> #include <memory><br>
><br>
> namespace llvm {<br>
><br>
> +class MIRParserImpl;<br>
> class SMDiagnostic;<br>
<span class="">><br>
> +/// This class initializes machine functions by applying the state loaded from<br>
> +/// a MIR file.<br>
</span>> +class MIRParser : public MachineFunctionInitializer {<br>
<span class="">> + std::unique_ptr<MIRParserImpl> Impl;<br>
> +<br>
</span><span class="">> +public:<br>
> + MIRParser(std::unique_ptr<MIRParserImpl> Impl);<br>
> + MIRParser(const MIRParser &) = delete;<br>
> + ~MIRParser();<br>
> +<br>
</span>> + /// Parse the optional LLVM IR module that's embedded in the MIR file.<br>
> + ///<br>
> + /// A new, empty module is created if the LLVM IR isn't present.<br>
> + /// Returns null if a parsing error occurred.<br>
> + std::unique_ptr<Module> parseLLVMModule(SMDiagnostic &Error);<br>
<br>
Make this std::unique_ptr<Module> parseLLVMModule();<br>
<span class=""><br>
> +<br>
> + /// Initialize the machine function to the state that's described in the MIR<br>
> + /// file.<br>
> + ///<br>
> + /// Return true if error occurred.<br>
> + bool initializeMachineFunction(MachineFunction &MF) override;<br>
> +};<br>
> +<br>
</span><span class="">> /// This function is the main interface to the MIR serialization format parser.<br>
> ///<br>
</span>> -/// It reads a YAML file that has an optional LLVM IR and returns an LLVM<br>
> -/// module.<br>
> +/// It reads in a MIR file and returns a MIR parser that can parse the embedded<br>
> +/// LLVM IR module and initialize the machine functions by parsing the machine<br>
> +/// function's state.<br>
> +///<br>
<span class="">> /// \param Filename - The name of the file to parse.<br>
> /// \param Error - Error result info.<br>
</span>> -/// \param Context - Context in which to allocate globals info.<br>
<span class="">> -std::unique_ptr<Module> parseMIRFile(StringRef Filename, SMDiagnostic &Error,<br>
> - LLVMContext &Context);<br>
</span>> +/// \param Context - Context which will be used for the parsed LLVM IR module.<br>
> +std::unique_ptr<MIRParser> createMIRParserFromFile(StringRef Filename,<br>
<span class="">> + SMDiagnostic &Error,<br>
> + LLVMContext &Context);<br>
><br>
> /// This function is another interface to the MIR serialization format parser.<br>
> ///<br>
</span>> -/// It parses the optional LLVM IR in the given buffer, and returns an LLVM<br>
> -/// module.<br>
> +/// It returns a MIR parser that works with the given memory buffer and that can<br>
> +/// parse the embedded LLVM IR module and initialize the machine functions by<br>
> +/// parsing the machine function's state.<br>
> +///<br>
<span class="">> /// \param Contents - The MemoryBuffer containing the machine level IR.<br>
</span>> -/// \param Error - Error result info.<br>
> -/// \param Context - Context in which to allocate globals info.<br>
<span class="">> -std::unique_ptr<Module> parseMIR(std::unique_ptr<MemoryBuffer> Contents,<br>
> - SMDiagnostic &Error, LLVMContext &Context);<br>
</span>> +/// \param Context - Context which will be used for the parsed LLVM IR module.<br>
> +std::unique_ptr<MIRParser><br>
> +createMIRParser(std::unique_ptr<MemoryBuffer> Contents, LLVMContext &Context);<br>
<span class="">><br>
> } // end namespace llvm<br>
><br>
> Index: include/llvm/CodeGen/MachineFunctionAnalysis.h<br>
> ===================================================================<br>
> --- include/llvm/CodeGen/MachineFunctionAnalysis.h<br>
> +++ include/llvm/CodeGen/MachineFunctionAnalysis.h<br>
> @@ -19,6 +19,7 @@<br>
> namespace llvm {<br>
><br>
> class MachineFunction;<br>
</span>> +class MachineFunctionInitializer;<br>
<span class="">> class TargetMachine;<br>
><br>
> /// MachineFunctionAnalysis - This class is a Pass that manages a<br>
> @@ -28,9 +29,12 @@<br>
> const TargetMachine &TM;<br>
> MachineFunction *MF;<br>
> unsigned NextFnNum;<br>
</span>> + MachineFunctionInitializer *MFInitializer;<br>
<span class="">> +<br>
> public:<br>
> static char ID;<br>
> - explicit MachineFunctionAnalysis(const TargetMachine &tm);<br>
</span>> + explicit MachineFunctionAnalysis(const TargetMachine &tm,<br>
> + MachineFunctionInitializer *MFInitializer);<br>
<span class="">> ~MachineFunctionAnalysis() override;<br>
><br>
> MachineFunction &getMF() const { return *MF; }<br>
</span>> Index: include/llvm/CodeGen/MachineFunctionInitializer.h<br>
> ===================================================================<br>
> --- /dev/null<br>
> +++ include/llvm/CodeGen/MachineFunctionInitializer.h<br>
> @@ -0,0 +1,38 @@<br>
> +//===- MachineFunctionInitalizer.h - machine function initializer ---------===//<br>
<span class="">> +//<br>
> +// The LLVM Compiler Infrastructure<br>
> +//<br>
> +// This file is distributed under the University of Illinois Open Source<br>
> +// License. See LICENSE.TXT for details.<br>
> +//<br>
> +//===----------------------------------------------------------------------===//<br>
> +//<br>
</span>> +// This file declares an interface that allows custom machine function<br>
> +// initialization.<br>
> +//<br>
> +//===----------------------------------------------------------------------===//<br>
> +<br>
> +#ifndef LLVM_CODEGEN_MACHINEFUNCTIONINITIALIZER_H<br>
> +#define LLVM_CODEGEN_MACHINEFUNCTIONINITIALIZER_H<br>
<span class="">> +<br>
> +namespace llvm {<br>
> +<br>
> +class MachineFunction;<br>
> +<br>
</span>> +/// This interface provides a way to initialize machine functions after they are<br>
> +/// created by the machine function analysis pass.<br>
> +class MachineFunctionInitializer {<br>
> + virtual void anchor();<br>
> +<br>
> +public:<br>
> + virtual ~MachineFunctionInitializer() {}<br>
> +<br>
> + /// Initialize the machine function.<br>
<span class="">> + ///<br>
> + /// Return true if error occurred.<br>
> + virtual bool initializeMachineFunction(MachineFunction &MF) = 0;<br>
</span><span class="">> +};<br>
> +<br>
> +} // end namespace llvm<br>
> +<br>
> +#endif<br>
</span>> Index: include/llvm/IR/DiagnosticInfo.h<br>
> ===================================================================<br>
> --- include/llvm/IR/DiagnosticInfo.h<br>
> +++ include/llvm/IR/DiagnosticInfo.h<br>
> @@ -32,6 +32,7 @@<br>
> class Twine;<br>
> class Value;<br>
> class DebugLoc;<br>
> +class SMDiagnostic;<br>
><br>
> /// \brief Defines the different supported severity of a diagnostic.<br>
> enum DiagnosticSeverity {<br>
> @@ -56,6 +57,7 @@<br>
> DK_OptimizationRemarkMissed,<br>
> DK_OptimizationRemarkAnalysis,<br>
> DK_OptimizationFailure,<br>
> + DK_MIRParser,<br>
> DK_FirstPluginKind<br>
> };<br>
><br>
> @@ -386,6 +388,24 @@<br>
> bool isEnabled() const override;<br>
> };<br>
><br>
> +/// Diagnostic information for machine IR parser.<br>
> +class DiagnosticInfoMIRParser : public DiagnosticInfo {<br>
> + const SMDiagnostic &Diagnostic;<br>
> +<br>
> +public:<br>
> + DiagnosticInfoMIRParser(DiagnosticSeverity Severity,<br>
> + const SMDiagnostic &Diagnostic)<br>
> + : DiagnosticInfo(DK_MIRParser, Severity), Diagnostic(Diagnostic) {}<br>
<br>
We can probably do with just storing "std::string Message" here for now<br>
, instead of a reference to an SMDiagnostic. Maybe also the .mir<br>
filename if that's appropriate. We can add more structure later if we<br>
need it.<br></blockquote><div><br></div><div>SMDiagnostics print colors, and they a nice and consistent look when printed, which shows the line and</div><div>column number before the 'error: ' indicator. They also print the pointer which shows you where the error is</div><div>and can print the appropriate source ranges as well. This is really important for me, as a lot of the errors that</div><div>will come later rely on all of those features.</div><div><br></div><div>I can't just print out SMDiagnostics to a string and store the string in the DiagnosticInfo - the color information</div><div>isn't preserved, and the default printer for the diagnostic infos prints the 'error: ' indicator before the message </div><div>which would break the consistency if my message would contain the line and column numbers. </div><div><br></div><div>This is the reason why it's important for me to store the SMDiagnostic and to print it out from the custom diagnostic</div><div>info handler in llc directly to errs().</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> +<br>
> + const SMDiagnostic &getDiagnostic() const { return Diagnostic; }<br>
> +<br>
> + void print(DiagnosticPrinter &DP) const override;<br>
> +<br>
> + static bool classof(const DiagnosticInfo *DI) {<br>
> + return DI->getKind() == DK_MIRParser;<br>
> + }<br>
> +};<br>
> +<br>
> // Create wrappers for C Binding types (see CBindingWrapping.h).<br>
> DEFINE_SIMPLE_CONVERSION_FUNCTIONS(DiagnosticInfo, LLVMDiagnosticInfoRef)<br>
<span class="">><br>
> Index: include/llvm/Target/TargetMachine.h<br>
> ===================================================================<br>
> --- include/llvm/Target/TargetMachine.h<br>
> +++ include/llvm/Target/TargetMachine.h<br>
> @@ -27,6 +27,7 @@<br>
> class InstrItineraryData;<br>
> class GlobalValue;<br>
> class Mangler;<br>
</span>> +class MachineFunctionInitializer;<br>
<span class="">> class MCAsmInfo;<br>
> class MCCodeGenInfo;<br>
> class MCContext;<br>
> @@ -208,11 +209,11 @@<br>
> /// emitted. Typically this will involve several steps of code generation.<br>
> /// This method should return true if emission of this file type is not<br>
> /// supported, or false on success.<br>
> - virtual bool addPassesToEmitFile(PassManagerBase &, raw_pwrite_stream &,<br>
> - CodeGenFileType,<br>
> - bool /*DisableVerify*/ = true,<br>
> - AnalysisID /*StartAfter*/ = nullptr,<br>
> - AnalysisID /*StopAfter*/ = nullptr) {<br>
> + virtual bool addPassesToEmitFile(<br>
> + PassManagerBase &, raw_pwrite_stream &, CodeGenFileType,<br>
> + bool /*DisableVerify*/ = true, AnalysisID /*StartAfter*/ = nullptr,<br>
> + AnalysisID /*StopAfter*/ = nullptr,<br>
</span>> + MachineFunctionInitializer * /*MFInitializer*/ = nullptr) {<br>
<span class="">> return true;<br>
> }<br>
><br>
> @@ -256,10 +257,11 @@<br>
><br>
> /// Add passes to the specified pass manager to get the specified file<br>
> /// emitted. Typically this will involve several steps of code generation.<br>
> - bool addPassesToEmitFile(PassManagerBase &PM, raw_pwrite_stream &Out,<br>
> - CodeGenFileType FileType, bool DisableVerify = true,<br>
> - AnalysisID StartAfter = nullptr,<br>
> - AnalysisID StopAfter = nullptr) override;<br>
> + bool addPassesToEmitFile(<br>
> + PassManagerBase &PM, raw_pwrite_stream &Out, CodeGenFileType FileType,<br>
> + bool DisableVerify = true, AnalysisID StartAfter = nullptr,<br>
> + AnalysisID StopAfter = nullptr,<br>
</span>> + MachineFunctionInitializer *MFInitializer = nullptr) override;<br>
<span class="">><br>
> /// Add passes to the specified pass manager to get machine code emitted with<br>
> /// the MCJIT. This method returns true if machine code is not supported. It<br>
> Index: lib/CodeGen/LLVMTargetMachine.cpp<br>
> ===================================================================<br>
> --- lib/CodeGen/LLVMTargetMachine.cpp<br>
> +++ lib/CodeGen/LLVMTargetMachine.cpp<br>
</span>> @@ -87,11 +87,11 @@<br>
<span class="">> }<br>
><br>
> /// addPassesToX helper drives creation and initialization of TargetPassConfig.<br>
> -static MCContext *addPassesToGenerateCode(LLVMTargetMachine *TM,<br>
> - PassManagerBase &PM,<br>
> - bool DisableVerify,<br>
> - AnalysisID StartAfter,<br>
> - AnalysisID StopAfter) {<br>
> +static MCContext *<br>
</span>> +addPassesToGenerateCode(LLVMTargetMachine *TM, PassManagerBase &PM,<br>
<span class="">> + bool DisableVerify, AnalysisID StartAfter,<br>
</span>> + AnalysisID StopAfter,<br>
> + MachineFunctionInitializer *MFInitializer = nullptr) {<br>
<span class="">><br>
> // Add internal analysis passes from the target machine.<br>
> PM.add(createTargetTransformInfoWrapperPass(TM->getTargetIRAnalysis()));<br>
</span>> @@ -121,7 +121,7 @@<br>
<span class="">> PM.add(MMI);<br>
><br>
> // Set up a MachineFunction for the rest of CodeGen to work on.<br>
> - PM.add(new MachineFunctionAnalysis(*TM));<br>
> + PM.add(new MachineFunctionAnalysis(*TM, MFInitializer));<br>
><br>
> // Enable FastISel with -fast, but allow that to be overridden.<br>
> if (EnableFastISelOption == cl::BOU_TRUE ||<br>
</span>> @@ -142,10 +142,11 @@<br>
<span class="">><br>
> bool LLVMTargetMachine::addPassesToEmitFile(<br>
> PassManagerBase &PM, raw_pwrite_stream &Out, CodeGenFileType FileType,<br>
> - bool DisableVerify, AnalysisID StartAfter, AnalysisID StopAfter) {<br>
> + bool DisableVerify, AnalysisID StartAfter, AnalysisID StopAfter,<br>
</span>> + MachineFunctionInitializer *MFInitializer) {<br>
<span class="">> // Add common CodeGen passes.<br>
> - MCContext *Context = addPassesToGenerateCode(this, PM, DisableVerify,<br>
> - StartAfter, StopAfter);<br>
> + MCContext *Context = addPassesToGenerateCode(<br>
> + this, PM, DisableVerify, StartAfter, StopAfter, MFInitializer);<br>
> if (!Context)<br>
> return true;<br>
><br>
> Index: lib/CodeGen/MIRParser/MIRParser.cpp<br>
> ===================================================================<br>
> --- lib/CodeGen/MIRParser/MIRParser.cpp<br>
> +++ lib/CodeGen/MIRParser/MIRParser.cpp<br>
</span>> @@ -14,9 +14,13 @@<br>
<span class="">><br>
> #include "llvm/CodeGen/MIRParser/MIRParser.h"<br>
> #include "llvm/ADT/StringRef.h"<br>
> +#include "llvm/ADT/StringMap.h"<br>
> #include "llvm/ADT/STLExtras.h"<br>
> #include "llvm/AsmParser/Parser.h"<br>
> +#include "llvm/CodeGen/MachineFunction.h"<br>
> #include "llvm/CodeGen/MIRYamlMapping.h"<br>
</span>> +#include "llvm/IR/DiagnosticInfo.h"<br>
> +#include "llvm/IR/LLVMContext.h"<br>
> #include "llvm/IR/Module.h"<br>
> #include "llvm/Support/LineIterator.h"<br>
> #include "llvm/Support/SMLoc.h"<br>
> @@ -27,19 +31,27 @@<br>
<span class="">><br>
> using namespace llvm;<br>
><br>
> -namespace {<br>
> +namespace llvm {<br>
><br>
> /// This class implements the parsing of LLVM IR that's embedded inside a MIR<br>
> /// file.<br>
> class MIRParserImpl {<br>
> SourceMgr SM;<br>
> StringRef Filename;<br>
> LLVMContext &Context;<br>
> + StringMap<std::unique_ptr<yaml::MachineFunction>> Functions;<br>
><br>
> public:<br>
> MIRParserImpl(std::unique_ptr<MemoryBuffer> Contents, StringRef Filename,<br>
</span>> LLVMContext &Context);<br>
><br>
> + void reportDiagnostic(const SMDiagnostic &Diag);<br>
> +<br>
> + /// Report an error at the given message.<br>
> + ///<br>
> + /// Always returns true.<br>
> + bool error(const Twine &Message);<br>
<br>
Why return anything at all if it's always true?</blockquote><div><br></div><div>To allow the parsing methods to have simple early exits on errors, like:</div><div><br></div><div> if (/* error condition */)</div><div> return error("error message");</div><div><br></div><div>vs</div><div> </div><div> if (/* error condition */) {</div><div> error("error message);</div><div> return true;</div><div> }</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> +<br>
> /// Try to parse the optional LLVM module and the machine functions in the MIR<br>
> /// file.<br>
> ///<br>
> @@ -51,27 +63,55 @@<br>
<span class="">> /// Return true if an error occurred.<br>
> bool parseMachineFunction(yaml::Input &In);<br>
><br>
> + /// Initialize the machine function to the state that's described in the MIR<br>
> + /// file.<br>
> + ///<br>
> + /// Return true if error occurred.<br>
</span>> + bool initializeMachineFunction(MachineFunction &MF);<br>
<span class="">> +<br>
> private:<br>
> /// Return a MIR diagnostic converted from an LLVM assembly diagnostic.<br>
> SMDiagnostic diagFromLLVMAssemblyDiag(const SMDiagnostic &Error,<br>
> SMRange SourceRange);<br>
> };<br>
><br>
> -} // end anonymous namespace<br>
> +} // end namespace llvm<br>
><br>
> MIRParserImpl::MIRParserImpl(std::unique_ptr<MemoryBuffer> Contents,<br>
> StringRef Filename, LLVMContext &Context)<br>
</span>> : SM(), Filename(Filename), Context(Context) {<br>
> SM.AddNewSourceBuffer(std::move(Contents), SMLoc());<br>
> }<br>
><br>
> +bool MIRParserImpl::error(const Twine &Message) {<br>
> + Context.diagnose(DiagnosticInfoMIRParser(<br>
> + DS_Error, SMDiagnostic(Filename, SourceMgr::DK_Error, Message.str())));<br>
<span class="">> + return true;<br>
> +}<br>
> +<br>
</span>> +void MIRParserImpl::reportDiagnostic(const SMDiagnostic &Diag) {<br>
> + DiagnosticSeverity Kind;<br>
> + switch (Diag.getKind()) {<br>
> + case SourceMgr::DK_Error:<br>
> + Kind = DS_Error;<br>
> + break;<br>
> + case SourceMgr::DK_Warning:<br>
> + Kind = DS_Warning;<br>
> + break;<br>
> + case SourceMgr::DK_Note:<br>
> + Kind = DS_Note;<br>
> + break;<br>
> + }<br>
<br>
Pull out the error message here and give that to diagnose.<br>
<br>
> + Context.diagnose(DiagnosticInfoMIRParser(Kind, Diag));<br>
> +}<br>
> +<br>
> static void handleYAMLDiag(const SMDiagnostic &Diag, void *Context) {<br>
> - *reinterpret_cast<SMDiagnostic *>(Context) = Diag;<br>
> + reinterpret_cast<MIRParserImpl *>(Context)->reportDiagnostic(Diag);<br>
> }<br>
><br>
> std::unique_ptr<Module> MIRParserImpl::parse(SMDiagnostic &Error) {<br>
> yaml::Input In(SM.getMemoryBuffer(SM.getMainFileID())->getBuffer(),<br>
> - /*Ctxt=*/nullptr, handleYAMLDiag, &Error);<br>
> + /*Ctxt=*/nullptr, handleYAMLDiag, this);<br>
<br>
A couple of things in here will need to call diagnose instead of<br>
populating Error and returning nullptr.<br>
<br>
><br>
> if (!In.setCurrentDocument()) {<br>
> if (!Error.getMessage().empty())<br>
> @@ -110,12 +150,21 @@<br>
<span class="">> }<br>
><br>
> bool MIRParserImpl::parseMachineFunction(yaml::Input &In) {<br>
> - yaml::MachineFunction MF;<br>
> - yaml::yamlize(In, MF, false);<br>
> + auto MF = llvm::make_unique<yaml::MachineFunction>();<br>
> + yaml::yamlize(In, *MF, false);<br>
> if (In.error())<br>
> return true;<br>
> - // TODO: Initialize the real machine function with the state in the yaml<br>
> - // machine function later on.<br>
> + auto FunctionName = MF->Name;<br>
> + Functions.insert(std::make_pair(FunctionName, std::move(MF)));<br>
> + return false;<br>
> +}<br>
> +<br>
</span>> +bool MIRParserImpl::initializeMachineFunction(MachineFunction &MF) {<br>
<span class="">> + auto It = Functions.find(MF.getName());<br>
> + if (It == Functions.end())<br>
</span>> + return error(Twine("no machine function information for function '") +<br>
<span class="">> + MF.getName() + "' in the MIR file");<br>
</span>> + // TODO: Recreate the machine function.<br>
> return false;<br>
> }<br>
><br>
> @@ -150,22 +199,35 @@<br>
<span class="">> Error.getFixIts());<br>
> }<br>
><br>
> -std::unique_ptr<Module> llvm::parseMIRFile(StringRef Filename,<br>
> - SMDiagnostic &Error,<br>
> - LLVMContext &Context) {<br>
> +MIRParser::MIRParser(std::unique_ptr<MIRParserImpl> Impl)<br>
> + : Impl(std::move(Impl)) {}<br>
> +<br>
> +MIRParser::~MIRParser() {}<br>
> +<br>
</span>> +std::unique_ptr<Module> MIRParser::parseLLVMModule(SMDiagnostic &Error) {<br>
> + return Impl->parse(Error);<br>
<span class="">> +}<br>
> +<br>
> +bool MIRParser::initializeMachineFunction(MachineFunction &MF) {<br>
</span>> + return Impl->initializeMachineFunction(MF);<br>
> +}<br>
> +<br>
> +std::unique_ptr<MIRParser> llvm::createMIRParserFromFile(StringRef Filename,<br>
<span class="">> + SMDiagnostic &Error,<br>
> + LLVMContext &Context) {<br>
> auto FileOrErr = MemoryBuffer::getFile(Filename);<br>
> if (std::error_code EC = FileOrErr.getError()) {<br>
> Error = SMDiagnostic(Filename, SourceMgr::DK_Error,<br>
> "Could not open input file: " + EC.message());<br>
> - return std::unique_ptr<Module>();<br>
</span>> + return nullptr;<br>
> }<br>
> - return parseMIR(std::move(FileOrErr.get()), Error, Context);<br>
> + return createMIRParser(std::move(FileOrErr.get()), Context);<br>
<span class="">> }<br>
><br>
> -std::unique_ptr<Module> llvm::parseMIR(std::unique_ptr<MemoryBuffer> Contents,<br>
> - SMDiagnostic &Error,<br>
> - LLVMContext &Context) {<br>
</span>> +std::unique_ptr<MIRParser><br>
> +llvm::createMIRParser(std::unique_ptr<MemoryBuffer> Contents,<br>
<span class="">> + LLVMContext &Context) {<br>
> auto Filename = Contents->getBufferIdentifier();<br>
> - MIRParserImpl Parser(std::move(Contents), Filename, Context);<br>
> - return Parser.parse(Error);<br>
</span>> + return llvm::make_unique<MIRParser>(<br>
> + llvm::make_unique<MIRParserImpl>(std::move(Contents), Filename, Context));<br>
> }<br>
> Index: lib/CodeGen/MachineFunction.cpp<br>
> ===================================================================<br>
> --- lib/CodeGen/MachineFunction.cpp<br>
> +++ lib/CodeGen/MachineFunction.cpp<br>
> @@ -19,6 +19,7 @@<br>
> #include "llvm/Analysis/ConstantFolding.h"<br>
> #include "llvm/CodeGen/MachineConstantPool.h"<br>
> #include "llvm/CodeGen/MachineFrameInfo.h"<br>
> +#include "llvm/CodeGen/MachineFunctionInitializer.h"<br>
> #include "llvm/CodeGen/MachineFunctionPass.h"<br>
> #include "llvm/CodeGen/MachineInstr.h"<br>
> #include "llvm/CodeGen/MachineJumpTableInfo.h"<br>
> @@ -41,6 +42,8 @@<br>
><br>
> #define DEBUG_TYPE "codegen"<br>
><br>
> +void MachineFunctionInitializer::anchor() {}<br>
> +<br>
> //===----------------------------------------------------------------------===//<br>
> // MachineFunction implementation<br>
> //===----------------------------------------------------------------------===//<br>
<span class="">> Index: lib/CodeGen/MachineFunctionAnalysis.cpp<br>
> ===================================================================<br>
> --- lib/CodeGen/MachineFunctionAnalysis.cpp<br>
> +++ lib/CodeGen/MachineFunctionAnalysis.cpp<br>
> @@ -15,12 +15,14 @@<br>
> #include "llvm/CodeGen/GCMetadata.h"<br>
> #include "llvm/CodeGen/MachineFunction.h"<br>
> #include "llvm/CodeGen/MachineModuleInfo.h"<br>
</span>> +#include "llvm/CodeGen/MachineFunctionInitializer.h"<br>
<span class="">> using namespace llvm;<br>
><br>
> char MachineFunctionAnalysis::ID = 0;<br>
><br>
> -MachineFunctionAnalysis::MachineFunctionAnalysis(const TargetMachine &tm) :<br>
> - FunctionPass(ID), TM(tm), MF(nullptr) {<br>
> +MachineFunctionAnalysis::MachineFunctionAnalysis(<br>
</span>> + const TargetMachine &tm, MachineFunctionInitializer *MFInitializer)<br>
<span class="">> + : FunctionPass(ID), TM(tm), MF(nullptr), MFInitializer(MFInitializer) {<br>
> initializeMachineModuleInfoPass(*PassRegistry::getPassRegistry());<br>
> }<br>
><br>
> @@ -47,6 +49,8 @@<br>
> assert(!MF && "MachineFunctionAnalysis already initialized!");<br>
> MF = new MachineFunction(&F, TM, NextFnNum++,<br>
> getAnalysis<MachineModuleInfo>());<br>
> + if (MFInitializer)<br>
> + MFInitializer->initializeMachineFunction(*MF);<br>
> return false;<br>
> }<br>
><br>
</span>> Index: lib/IR/DiagnosticInfo.cpp<br>
> ===================================================================<br>
> --- lib/IR/DiagnosticInfo.cpp<br>
> +++ lib/IR/DiagnosticInfo.cpp<br>
> @@ -25,6 +25,7 @@<br>
> #include "llvm/Support/Atomic.h"<br>
> #include "llvm/Support/CommandLine.h"<br>
> #include "llvm/Support/Regex.h"<br>
> +#include "llvm/Support/SourceMgr.h"<br>
> #include <string><br>
><br>
> using namespace llvm;<br>
> @@ -170,6 +171,13 @@<br>
> PassRemarksAnalysisOptLoc.Pattern->match(getPassName());<br>
> }<br>
><br>
> +void DiagnosticInfoMIRParser::print(DiagnosticPrinter &DP) const {<br>
> + std::string Str;<br>
> + raw_string_ostream StrOS(Str);<br>
> + Diagnostic.print("", StrOS);<br>
> + DP << StrOS.str();<br>
> +}<br>
> +<br>
> void llvm::emitOptimizationRemark(LLVMContext &Ctx, const char *PassName,<br>
> const Function &Fn, const DebugLoc &DLoc,<br>
> const Twine &Msg) {<br>
<span class="">> Index: lib/Target/CppBackend/CPPBackend.cpp<br>
> ===================================================================<br>
> --- lib/Target/CppBackend/CPPBackend.cpp<br>
> +++ lib/Target/CppBackend/CPPBackend.cpp<br>
> @@ -2148,7 +2148,8 @@<br>
><br>
> bool CPPTargetMachine::addPassesToEmitFile(<br>
> PassManagerBase &PM, raw_pwrite_stream &o, CodeGenFileType FileType,<br>
> - bool DisableVerify, AnalysisID StartAfter, AnalysisID StopAfter) {<br>
> + bool DisableVerify, AnalysisID StartAfter, AnalysisID StopAfter,<br>
</span>> + MachineFunctionInitializer *MFInitializer) {<br>
<span class="">> if (FileType != TargetMachine::CGFT_AssemblyFile)<br>
> return true;<br>
> auto FOut = llvm::make_unique<formatted_raw_ostream>(o);<br>
> Index: lib/Target/CppBackend/CPPTargetMachine.h<br>
> ===================================================================<br>
> --- lib/Target/CppBackend/CPPTargetMachine.h<br>
> +++ lib/Target/CppBackend/CPPTargetMachine.h<br>
</span>> @@ -31,8 +31,8 @@<br>
> public:<br>
<span class="">> bool addPassesToEmitFile(PassManagerBase &PM, raw_pwrite_stream &Out,<br>
</span><span class="">> CodeGenFileType FileType, bool DisableVerify,<br>
> - AnalysisID StartAfter,<br>
> - AnalysisID StopAfter) override;<br>
</span>> + AnalysisID StartAfter, AnalysisID StopAfter,<br>
> + MachineFunctionInitializer *MFInitializer) override;<br>
<div><div class="h5">> };<br>
><br>
> extern Target TheCppBackendTarget;<br>
> Index: test/CodeGen/MIR/function-missing-machine-function.mir<br>
> ===================================================================<br>
> --- /dev/null<br>
> +++ test/CodeGen/MIR/function-missing-machine-function.mir<br>
> @@ -0,0 +1,13 @@<br>
> +# RUN: not llc -start-after branch-folder -stop-after branch-folder -o /dev/null %s 2>&1 | FileCheck %s<br>
> +# This test verifies that an error is reported when a MIR file has some<br>
> +# function but is missing a corresponding machine function.<br>
> +<br>
> +# CHECK: no machine function information for function 'foo' in the MIR file<br>
> +<br>
> +--- |<br>
> +<br>
> + define i32 @foo() {<br>
> + ret i32 0<br>
> + }<br>
> +<br>
> +...<br>
> Index: test/CodeGen/MIR/llvmIR.mir<br>
> ===================================================================<br>
> --- test/CodeGen/MIR/llvmIR.mir<br>
> +++ test/CodeGen/MIR/llvmIR.mir<br>
> @@ -30,3 +30,6 @@<br>
> }<br>
><br>
> ...<br>
> +---<br>
> +name: foo<br>
> +...<br>
> Index: tools/llc/llc.cpp<br>
> ===================================================================<br>
> --- tools/llc/llc.cpp<br>
> +++ tools/llc/llc.cpp<br>
</div></div>> @@ -22,6 +22,7 @@<br>
> #include "llvm/CodeGen/LinkAllCodegenComponents.h"<br>
> #include "llvm/CodeGen/MIRParser/MIRParser.h"<br>
> #include "llvm/IR/DataLayout.h"<br>
> +#include "llvm/IR/DiagnosticInfo.h"<br>
> #include "llvm/IR/IRPrintingPasses.h"<br>
> #include "llvm/IR/LLVMContext.h"<br>
> #include "llvm/IR/LegacyPassManager.h"<br>
> @@ -166,6 +167,22 @@<br>
> return FDOut;<br>
> }<br>
><br>
> +void handleLLVMDiagnostic(const DiagnosticInfo &DI, void *C) {<br>
> + LLVMContext &Context = *reinterpret_cast<LLVMContext *>(C);<br>
> +<br>
> + if (const auto *MIRDiagnostic = cast<DiagnosticInfoMIRParser>(&DI)) {<br>
> + MIRDiagnostic->getDiagnostic().print("", errs());<br>
> + if (MIRDiagnostic->getSeverity() != DS_Error)<br>
> + return;<br>
> + exit(1);<br>
> + }<br>
> +<br>
> + // Redirect the diagnostic back to the LLVM Context<br>
> + Context.setDiagnosticHandler(nullptr);<br>
> + Context.diagnose(DI);<br>
> + Context.setDiagnosticHandler(handleLLVMDiagnostic, C);<br>
> +}<br>
<br>
This does nothing at all - you're reimplementing the behaviour you'd get<br>
if you didn't set a diagnostic handler, and falling back to the default<br>
by changing global state is error prone and ugly. Just remove this.</blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> +<br>
> // main - Entry point for the llc compiler.<br>
> //<br>
> int main(int argc, char **argv) {<br>
> @@ -176,6 +193,7 @@<br>
> EnableDebugBuffering = true;<br>
><br>
> LLVMContext &Context = getGlobalContext();<br>
> + Context.setDiagnosticHandler(handleLLVMDiagnostic, &Context);<br>
> llvm_shutdown_obj Y; // Call llvm_shutdown() on exit.<br>
><br>
> // Initialize targets first, so that --version shows registered targets.<br>
> @@ -210,16 +228,19 @@<br>
<span class="">> // Load the module to be compiled...<br>
> SMDiagnostic Err;<br>
> std::unique_ptr<Module> M;<br>
> + std::unique_ptr<MIRParser> MIR;<br>
> Triple TheTriple;<br>
><br>
> bool SkipModule = MCPU == "help" ||<br>
> (!MAttrs.empty() && MAttrs.front() == "help");<br>
><br>
> // If user just wants to list available options, skip module loading<br>
> if (!SkipModule) {<br>
> - if (StringRef(InputFilename).endswith_lower(".mir"))<br>
> - M = parseMIRFile(InputFilename, Err, Context);<br>
> - else<br>
> + if (StringRef(InputFilename).endswith_lower(".mir")) {<br>
</span>> + MIR = createMIRParserFromFile(InputFilename, Err, Context);<br>
> + if (MIR)<br>
> + M = MIR->parseLLVMModule(Err);<br>
<span class="">> + } else<br>
> M = parseIRFile(InputFilename, Err, Context);<br>
> if (!M) {<br>
> Err.print(argv[0], errs());<br>
<br>
</span>You'll have to restructure this a little to deal with the fact that<br>
parseLLVMModule doesn't fill Err anymore, or maybe just assert(M &&<br>
"parseLLVMModule exits on failure") after parseLLVMModule for now to<br>
document the implicit control flow.<br>
<br>
> @@ -350,7 +371,7 @@<br>
<div class="HOEnZb"><div class="h5">><br>
> // Ask the target to add backend passes as necessary.<br>
> if (Target->addPassesToEmitFile(PM, *OS, FileType, NoVerify, StartAfterID,<br>
> - StopAfterID)) {<br>
> + StopAfterID, MIR.get())) {<br>
> errs() << argv[0] << ": target does not support generation of this"<br>
> << " file type!\n";<br>
> return 1;<br>
</div></div></blockquote></div><br></div></div>