[PATCH] MIR Parser: use the correct, file based locations for machine instruction diagnostics.

Duncan P. N. Exon Smith dexonsmith at apple.com
Fri Jun 19 14:41:52 PDT 2015


> On 2015-Jun-19, at 13:49, Alex Lorenz <arphaman at gmail.com> wrote:
> 
> Hi dexonsmith, bob.wilson, bogner,
> 
> This patch is based on the patch that serializes immediate machine operands (http://reviews.llvm.org/D10573).
> 
> This patch translates the source locations for machine instruction parsing diagnostics from the locations in the
> machine instruction source string to the locations in the MIR file.
> 
> REPOSITORY
>  rL LLVM
> 
> http://reviews.llvm.org/D10574
> 
> Files:
>  include/llvm/CodeGen/MIRYamlMapping.h
>  lib/CodeGen/MIRParser/MIRParser.cpp
>  test/CodeGen/MIR/X86/expected-machine-operand.mir
>  test/CodeGen/MIR/X86/missing-comma.mir
>  test/CodeGen/MIR/X86/missing-instruction.mir
>  test/CodeGen/MIR/X86/unknown-instruction.mir
>  test/CodeGen/MIR/X86/unknown-register.mir
>  test/CodeGen/MIR/X86/unrecognized-character.mir
> 
> EMAIL PREFERENCES
>  http://reviews.llvm.org/settings/panel/emailpreferences/
> <D10574.28044.patch>

Awesome.  LGTM with a minor nitpick (that you're free to ignore).

> Index: lib/CodeGen/MIRParser/MIRParser.cpp
> ===================================================================
> --- lib/CodeGen/MIRParser/MIRParser.cpp
> +++ lib/CodeGen/MIRParser/MIRParser.cpp
> @@ -233,16 +238,32 @@
>    // Parse the instructions.
>    for (const auto &MISource : YamlMBB.Instructions) {
>      SMDiagnostic Error;
> -    if (auto *MI = parseMachineInstr(SM, MF, MISource, Error)) {
> +    if (auto *MI = parseMachineInstr(SM, MF, MISource.Value, Error)) {
>        MBB.insert(MBB.end(), MI);
>        continue;
>      }
> -    reportDiagnostic(Error);
> +    reportDiagnostic(diagFromMIStringDiag(Error, MISource.SourceRange));
>      return true;
>    }
>    return false;
>  }
>  
> +SMDiagnostic MIRParserImpl::diagFromMIStringDiag(const SMDiagnostic &Error,
> +                                                 SMRange SourceRange) {
> +  assert(SourceRange.isValid() && "Invalid source range");
> +  SMLoc Loc = SourceRange.Start;
> +  bool HasQuote = Loc.getPointer() < SourceRange.End.getPointer() &&
> +                  *Loc.getPointer() == '\'';
> +  // Translate the location of the error from the location in the MI string to
> +  // the corresponding location in the MIR file.
> +  Loc = Loc.getFromPointer(Loc.getPointer() + Error.getColumnNo() +
> +                           (HasQuote ? 1 : 0));

I think you can just use `HasQuote` directly here.

> +
> +  // TODO: Translate any source ranges as well.
> +  return SM.GetMessage(Loc, Error.getKind(), Error.getMessage(), None,
> +                       Error.getFixIts());
> +}
> +
>  SMDiagnostic MIRParserImpl::diagFromLLVMAssemblyDiag(const SMDiagnostic &Error,
>                                                       SMRange SourceRange) {
>    assert(SourceRange.isValid());





More information about the llvm-commits mailing list