[llvm] r294433 - [Assembler] Enable nicer diagnostics for inline assembly.
Diana Picus via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 8 06:16:48 PST 2017
Hi,
I reverted this because I think it broke the buildbots. See
http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15/builds/3703
The stripped-shared test was a separate issue, so I don't think the
bot sent any emails (it was already red).
Please recommit after you've fixed the test. Let me know if you need
any help debugging/reproducing it.
Sorry,
Diana
On 8 February 2017 at 11:20, Sanne Wouda via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: sanwou01
> Date: Wed Feb 8 04:20:07 2017
> New Revision: 294433
>
> URL: http://llvm.org/viewvc/llvm-project?rev=294433&view=rev
> Log:
> [Assembler] Enable nicer diagnostics for inline assembly.
>
> Summary:
> Enables source location in diagnostic messages from the backend. This
> is after parsing, during finalization. This requires the SourceMgr, the
> inline assembly string buffer, and DiagInfo to still be alive after
> EmitInlineAsm returns.
>
> This patch creates a single SourceMgr for inline assembly inside the
> AsmPrinter. MCContext gets a pointer to this SourceMgr. Using one
> SourceMgr per call to EmitInlineAsm would make it difficult for
> MCContext to figure out in which SourceMgr the SMLoc is located, while a
> single SourceMgr can figure it out if it has multiple buffers.
>
> The Str argument to EmitInlineAsm is copied into a buffer and owned by
> the inline asm SourceMgr. This ensures that DiagHandlers won't print
> garbage. (Clang emits a "note: instantiated into assembly here", which
> refers to this string.)
>
> The AsmParser gets destroyed before finalization, which means that the
> DiagHandlers the AsmParser installs into the SourceMgr will be stale.
> Restore the saved DiagHandlers.
>
> Since now we're using just one SourceMgr for multiple inline asm
> strings, we need to tell the AsmParser which buffer it needs to parse
> currently. Hand a buffer id -- returned from SourceMgr::
> AddNewSourceBuffer -- to the AsmParser.
>
> Reviewers: rnk, grosbach, compnerd, rengolin, rovka, anemet
>
> Reviewed By: rnk
>
> Subscribers: llvm-commits
>
> Differential Revision: https://reviews.llvm.org/D29441
>
> Added:
> llvm/trunk/test/Assembler/inline-asm-diags.ll
> Modified:
> llvm/trunk/include/llvm/CodeGen/AsmPrinter.h
> llvm/trunk/include/llvm/MC/MCContext.h
> llvm/trunk/include/llvm/MC/MCParser/MCAsmParser.h
> llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
> llvm/trunk/lib/MC/MCContext.cpp
> llvm/trunk/lib/MC/MCParser/AsmParser.cpp
>
> Modified: llvm/trunk/include/llvm/CodeGen/AsmPrinter.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/AsmPrinter.h?rev=294433&r1=294432&r2=294433&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/AsmPrinter.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/AsmPrinter.h Wed Feb 8 04:20:07 2017
> @@ -23,6 +23,7 @@
> #include "llvm/IR/InlineAsm.h"
> #include "llvm/Support/DataTypes.h"
> #include "llvm/Support/ErrorHandling.h"
> +#include "llvm/Support/SourceMgr.h"
>
> namespace llvm {
> class AsmPrinterHandler;
> @@ -137,6 +138,19 @@ private:
> /// maintains ownership of the emitters.
> SmallVector<HandlerInfo, 1> Handlers;
>
> +public:
> + struct SrcMgrDiagInfo {
> + SourceMgr SrcMgr;
> + const MDNode *LocInfo;
> + LLVMContext::InlineAsmDiagHandlerTy DiagHandler;
> + void *DiagContext;
> + };
> +
> +private:
> + /// Structure for generating diagnostics for inline assembly. Only initialised
> + /// when necessary.
> + mutable std::unique_ptr<SrcMgrDiagInfo> DiagInfo;
> +
> /// If the target supports dwarf debug info, this pointer is non-null.
> DwarfDebug *DD;
>
>
> Modified: llvm/trunk/include/llvm/MC/MCContext.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCContext.h?rev=294433&r1=294432&r2=294433&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/MC/MCContext.h (original)
> +++ llvm/trunk/include/llvm/MC/MCContext.h Wed Feb 8 04:20:07 2017
> @@ -59,6 +59,9 @@ namespace llvm {
> /// The SourceMgr for this object, if any.
> const SourceMgr *SrcMgr;
>
> + /// The SourceMgr for inline assembly, if any.
> + SourceMgr *InlineSrcMgr;
> +
> /// The MCAsmInfo for this target.
> const MCAsmInfo *MAI;
>
> @@ -243,6 +246,8 @@ namespace llvm {
>
> const SourceMgr *getSourceManager() const { return SrcMgr; }
>
> + void setInlineSourceManager(SourceMgr *SM) { InlineSrcMgr = SM; }
> +
> const MCAsmInfo *getAsmInfo() const { return MAI; }
>
> const MCRegisterInfo *getRegisterInfo() const { return MRI; }
>
> Modified: llvm/trunk/include/llvm/MC/MCParser/MCAsmParser.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCParser/MCAsmParser.h?rev=294433&r1=294432&r2=294433&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/MC/MCParser/MCAsmParser.h (original)
> +++ llvm/trunk/include/llvm/MC/MCParser/MCAsmParser.h Wed Feb 8 04:20:07 2017
> @@ -259,7 +259,7 @@ public:
>
> /// \brief Create an MCAsmParser instance.
> MCAsmParser *createMCAsmParser(SourceMgr &, MCContext &, MCStreamer &,
> - const MCAsmInfo &);
> + const MCAsmInfo &, unsigned CB = 0);
>
> } // end namespace llvm
>
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp?rev=294433&r1=294432&r2=294433&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp Wed Feb 8 04:20:07 2017
> @@ -40,19 +40,12 @@ using namespace llvm;
>
> #define DEBUG_TYPE "asm-printer"
>
> -namespace {
> - struct SrcMgrDiagInfo {
> - const MDNode *LocInfo;
> - LLVMContext::InlineAsmDiagHandlerTy DiagHandler;
> - void *DiagContext;
> - };
> -}
> -
> /// srcMgrDiagHandler - This callback is invoked when the SourceMgr for an
> /// inline asm has an error in it. diagInfo is a pointer to the SrcMgrDiagInfo
> /// struct above.
> static void srcMgrDiagHandler(const SMDiagnostic &Diag, void *diagInfo) {
> - SrcMgrDiagInfo *DiagInfo = static_cast<SrcMgrDiagInfo *>(diagInfo);
> + AsmPrinter::SrcMgrDiagInfo *DiagInfo =
> + static_cast<AsmPrinter::SrcMgrDiagInfo *>(diagInfo);
> assert(DiagInfo && "Diagnostic context not passed down?");
>
> // If the inline asm had metadata associated with it, pull out a location
> @@ -99,35 +92,34 @@ void AsmPrinter::EmitInlineAsm(StringRef
> return;
> }
>
> - SourceMgr SrcMgr;
> - SrcMgr.setIncludeDirs(MCOptions.IASSearchPaths);
> + if (!DiagInfo) {
> + DiagInfo = make_unique<SrcMgrDiagInfo>();
>
> - SrcMgrDiagInfo DiagInfo;
> + MCContext &Context = MMI->getContext();
> + Context.setInlineSourceManager(&DiagInfo->SrcMgr);
>
> - // If the current LLVMContext has an inline asm handler, set it in SourceMgr.
> - LLVMContext &LLVMCtx = MMI->getModule()->getContext();
> - bool HasDiagHandler = false;
> - if (LLVMCtx.getInlineAsmDiagnosticHandler() != nullptr) {
> - // If the source manager has an issue, we arrange for srcMgrDiagHandler
> - // to be invoked, getting DiagInfo passed into it.
> - DiagInfo.LocInfo = LocMDNode;
> - DiagInfo.DiagHandler = LLVMCtx.getInlineAsmDiagnosticHandler();
> - DiagInfo.DiagContext = LLVMCtx.getInlineAsmDiagnosticContext();
> - SrcMgr.setDiagHandler(srcMgrDiagHandler, &DiagInfo);
> - HasDiagHandler = true;
> + LLVMContext &LLVMCtx = MMI->getModule()->getContext();
> + if (LLVMCtx.getInlineAsmDiagnosticHandler()) {
> + DiagInfo->DiagHandler = LLVMCtx.getInlineAsmDiagnosticHandler();
> + DiagInfo->DiagContext = LLVMCtx.getInlineAsmDiagnosticContext();
> + DiagInfo->SrcMgr.setDiagHandler(srcMgrDiagHandler, DiagInfo.get());
> + }
> }
>
> + SourceMgr &SrcMgr = DiagInfo->SrcMgr;
> + SrcMgr.setIncludeDirs(MCOptions.IASSearchPaths);
> + DiagInfo->LocInfo = LocMDNode;
> +
> std::unique_ptr<MemoryBuffer> Buffer;
> - if (isNullTerminated)
> - Buffer = MemoryBuffer::getMemBuffer(Str, "<inline asm>");
> - else
> - Buffer = MemoryBuffer::getMemBufferCopy(Str, "<inline asm>");
> + // The inline asm source manager will outlive Str, so make a copy of the
> + // string for SourceMgr to own.
> + Buffer = MemoryBuffer::getMemBufferCopy(Str, "<inline asm>");
>
> // Tell SrcMgr about this buffer, it takes ownership of the buffer.
> - SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());
> + unsigned BufNum = SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());
>
> std::unique_ptr<MCAsmParser> Parser(
> - createMCAsmParser(SrcMgr, OutContext, *OutStreamer, *MAI));
> + createMCAsmParser(SrcMgr, OutContext, *OutStreamer, *MAI, BufNum));
>
> // We create a new MCInstrInfo here since we might be at the module level
> // and not have a MachineFunction to initialize the TargetInstrInfo from and
> @@ -151,7 +143,13 @@ void AsmPrinter::EmitInlineAsm(StringRef
> int Res = Parser->Run(/*NoInitialTextSection*/ true,
> /*NoFinalize*/ true);
> emitInlineAsmEnd(STI, &TAP->getSTI());
> - if (Res && !HasDiagHandler)
> +
> + // LocInfo cannot be used for error generation from the backend.
> + // FIXME: associate LocInfo with the SourceBuffer to improve backend
> + // messages.
> + DiagInfo->LocInfo = nullptr;
> +
> + if (Res && !DiagInfo->DiagHandler)
> report_fatal_error("Error parsing inline asm\n");
> }
>
>
> Modified: llvm/trunk/lib/MC/MCContext.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCContext.cpp?rev=294433&r1=294432&r2=294433&view=diff
> ==============================================================================
> --- llvm/trunk/lib/MC/MCContext.cpp (original)
> +++ llvm/trunk/lib/MC/MCContext.cpp Wed Feb 8 04:20:07 2017
> @@ -521,13 +521,15 @@ CodeViewContext &MCContext::getCVContext
> void MCContext::reportError(SMLoc Loc, const Twine &Msg) {
> HadError = true;
>
> - // If we have a source manager use it. Otherwise just use the generic
> - // report_fatal_error().
> - if (!SrcMgr)
> + // If we have a source manager use it. Otherwise, try using the inline source
> + // manager.
> + // If that fails, use the generic report_fatal_error().
> + if (SrcMgr)
> + SrcMgr->PrintMessage(Loc, SourceMgr::DK_Error, Msg);
> + else if (InlineSrcMgr)
> + InlineSrcMgr->PrintMessage(Loc, SourceMgr::DK_Error, Msg);
> + else
> report_fatal_error(Msg, false);
> -
> - // Use the source manager to print the message.
> - SrcMgr->PrintMessage(Loc, SourceMgr::DK_Error, Msg);
> }
>
> void MCContext::reportFatalError(SMLoc Loc, const Twine &Msg) {
>
> Modified: llvm/trunk/lib/MC/MCParser/AsmParser.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCParser/AsmParser.cpp?rev=294433&r1=294432&r2=294433&view=diff
> ==============================================================================
> --- llvm/trunk/lib/MC/MCParser/AsmParser.cpp (original)
> +++ llvm/trunk/lib/MC/MCParser/AsmParser.cpp Wed Feb 8 04:20:07 2017
> @@ -209,7 +209,7 @@ private:
>
> public:
> AsmParser(SourceMgr &SM, MCContext &Ctx, MCStreamer &Out,
> - const MCAsmInfo &MAI);
> + const MCAsmInfo &MAI, unsigned CB);
> ~AsmParser() override;
>
> bool Run(bool NoInitialTextSection, bool NoFinalize = false) override;
> @@ -572,9 +572,9 @@ extern MCAsmParserExtension *createCOFFA
> enum { DEFAULT_ADDRSPACE = 0 };
>
> AsmParser::AsmParser(SourceMgr &SM, MCContext &Ctx, MCStreamer &Out,
> - const MCAsmInfo &MAI)
> + const MCAsmInfo &MAI, unsigned CB = 0)
> : Lexer(MAI), Ctx(Ctx), Out(Out), MAI(MAI), SrcMgr(SM),
> - PlatformParser(nullptr), CurBuffer(SM.getMainFileID()),
> + PlatformParser(nullptr), CurBuffer(CB ? CB : SM.getMainFileID()),
> MacrosEnabledFlag(true), CppHashInfo(), AssemblerDialect(~0U),
> IsDarwin(false), ParsingInlineAsm(false) {
> HadError = false;
> @@ -608,6 +608,10 @@ AsmParser::AsmParser(SourceMgr &SM, MCCo
> AsmParser::~AsmParser() {
> assert((HadError || ActiveMacros.empty()) &&
> "Unexpected active macro instantiation!");
> +
> + // Restore the saved diagnostics handler and context for use during
> + // finalization.
> + SrcMgr.setDiagHandler(SavedDiagHandler, SavedDiagContext);
> }
>
> void AsmParser::printMacroInstantiations() {
> @@ -5520,6 +5524,7 @@ bool parseAssignmentExpression(StringRef
>
> /// \brief Create an MCAsmParser instance.
> MCAsmParser *llvm::createMCAsmParser(SourceMgr &SM, MCContext &C,
> - MCStreamer &Out, const MCAsmInfo &MAI) {
> - return new AsmParser(SM, C, Out, MAI);
> + MCStreamer &Out, const MCAsmInfo &MAI,
> + unsigned CB) {
> + return new AsmParser(SM, C, Out, MAI, CB);
> }
>
> Added: llvm/trunk/test/Assembler/inline-asm-diags.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Assembler/inline-asm-diags.ll?rev=294433&view=auto
> ==============================================================================
> --- llvm/trunk/test/Assembler/inline-asm-diags.ll (added)
> +++ llvm/trunk/test/Assembler/inline-asm-diags.ll Wed Feb 8 04:20:07 2017
> @@ -0,0 +1,9 @@
> +; RUN: not llc -filetype=obj < %s 2>&1 -o /dev/null | FileCheck %s
> +
> +module asm ".word 0x10"
> +module asm ".word -bar"
> +
> +; CHECK: <inline asm>:2:7: error: expected relocatable expression
> +
> +module asm ".word -foo"
> +; CHECK: <inline asm>:3:7: error: expected relocatable expression
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list