[PATCH] MIR Serialization: Report diagnostics with correct locations for LLVM IR Errors.

[Alex L]

Thanks,
I will change the patch accordingly and commit it later on.

Alex.

2015-05-28 16:34 GMT-07:00 Duncan P. N. Exon Smith <dexonsmith at apple.com>:

>
> > On 2015-May-28, at 15:52, Alex Lorenz <arphaman at gmail.com> wrote:
> >
> > Hi dexonsmith, bob.wilson, bogner,
> >
> > This patch translates the line and column numbers from LLVM IR block
> string to the
> > line and column numbers in the MIR file so that llc reports LLVM IR
> errors with
> > correct locations.
> >
> > REPOSITORY
> >  rL LLVM
> >
> > http://reviews.llvm.org/D10108
> >
> > Files:
> >  lib/CodeGen/MIRParser/MIRParser.cpp
> >  test/CodeGen/MIR/llvm-ir-error-reported.mir
> >
> > Index: lib/CodeGen/MIRParser/MIRParser.cpp
> > ===================================================================
> > --- lib/CodeGen/MIRParser/MIRParser.cpp
> > +++ lib/CodeGen/MIRParser/MIRParser.cpp
> > @@ -18,6 +18,7 @@
> >  #include "llvm/AsmParser/Parser.h"
> >  #include "llvm/CodeGen/MIRYamlMapping.h"
> >  #include "llvm/IR/Module.h"
> > +#include "llvm/Support/LineIterator.h"
> >  #include "llvm/Support/SMLoc.h"
> >  #include "llvm/Support/SourceMgr.h"
> >  #include "llvm/Support/MemoryBuffer.h"
> > @@ -49,6 +50,11 @@
> >    ///
> >    /// Return true if an error occurred.
> >    bool parseMachineFunction(yaml::Input &In);
> > +
> > +private:
> > +  /// Return a MIR diagnostic converted from an LLVM assembly
> diagnostic.
> > +  SMDiagnostic diagFromLLVMAssemblyDiag(const SMDiagnostic &Error,
> > +                                        SMRange SourceRange);
> >  };
> >
> >  } // end anonymous namespace
> > @@ -81,8 +87,10 @@
> >            dyn_cast_or_null<yaml::BlockScalarNode>(In.getCurrentNode()))
> {
> >      M = parseAssembly(MemoryBufferRef(BSN->getValue(), Filename), Error,
> >                        Context);
> > -    if (!M)
> > +    if (!M) {
> > +      Error = diagFromLLVMAssemblyDiag(Error, BSN->getSourceRange());
> >        return M;
> > +    }
> >      In.nextDocument();
> >      if (!In.setCurrentDocument())
> >        return M;
> > @@ -111,6 +119,36 @@
> >    return false;
> >  }
> >
> > +SMDiagnostic MIRParserImpl::diagFromLLVMAssemblyDiag(const SMDiagnostic
> &Error,
> > +                                                     SMRange
> SourceRange) {
> > +  assert(SourceRange.isValid());
> > +
> > +  // Translate the location of the error from the location in the llvm
> IR string
> > +  // to the corresponding location in the MIR file.
> > +  auto LineAndColumn = SM.getLineAndColumn(SourceRange.Start);
> > +  unsigned Line = LineAndColumn.first + Error.getLineNo() - 1;
> > +  unsigned Column = Error.getColumnNo();
> > +  StringRef LineStr = Error.getLineContents();
> > +  SMLoc Loc = Error.getLoc();
> > +
> > +  // Get the full line and adjust the column number by taking the
> indentation of
> > +  // LLVM IR into account.
> > +  line_iterator Lines(*SM.getMemoryBuffer(SM.getMainFileID()), false);
> > +  while (!Lines.is_at_end() && Lines.line_number() != Line)
> > +    ++Lines;
> > +  if (Lines.line_number() == Line) {
> > +    LineStr = *Lines;
> > +    Loc = SMLoc::getFromPointer(LineStr.data());
> > +    auto Indent = LineStr.find(Error.getLineContents());
> > +    if (Indent != StringRef::npos)
> > +      Column += Indent;
> > +  }
>
> According to the `line_iterator` docs, calling `line_number()` isn't
> safe at EOF.  Your `if` statement looks like it could violate that.
>
> I think you should change this to use a standard iterator pattern, and
> only use the iterator when it's valid:
>
>     for (line_iterator L(*SM.getMemoryBuffer(SM.getMainFileID()), false),
> E;
>          L != E; ++L)
>       if (L.line_number() == Line) {
>         LineStr = *L;
>         Loc = SMLoc::getFromPointer(LineStr.data());
>         auto Indent = LineStr.find(Error.getLineContents());
>         if (Indent != StringRef::npos)
>           Column += Indent;
>         break;
>       }
>
> With that change, this LGTM.
>
> > +
> > +  return SMDiagnostic(SM, Loc, Filename, Line, Column, Error.getKind(),
> > +                      Error.getMessage(), LineStr, Error.getRanges(),
> > +                      Error.getFixIts());
> > +}
> > +
> >  std::unique_ptr<Module> llvm::parseMIRFile(StringRef Filename,
> >                                             SMDiagnostic &Error,
> >                                             LLVMContext &Context) {
> > Index: test/CodeGen/MIR/llvm-ir-error-reported.mir
> > ===================================================================
> > --- /dev/null
> > +++ test/CodeGen/MIR/llvm-ir-error-reported.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 an error is reported if the embedded LLVM IR
> contains an
> > +# error.
> > +
> > +--- |
> > +
> > +  ; CHECK: [[@LINE+3]]:15: error: use of undefined value '%a'
> > +  define i32 @foo(i32 %x, i32 %y) {
> > +    %z = alloca i32, align 4
> > +    store i32 %a, i32* %z, align 4
> > +    br label %Test
> > +  Test:
> > +    %m = load i32, i32* %z, align 4
> > +    %cond = icmp eq i32 %y, %m
> > +    br i1 %cond, label %IfEqual, label %IfUnequal
> > +  IfEqual:
> > +    ret i32 1
> > +  IfUnequal:
> > +    ret i32 0
> > +  }
> > +
> > +...
> >
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150528/d67a4137/attachment.html>