This is an archive of the discontinued LLVM Phabricator instance.

[TableGen] AsmMatcher: Fix bug with reported diagnostic for operand.
ClosedPublic

Authored by sdesmalen on Nov 14 2017, 2:50 AM.

Details

Summary

The generated diagnostic by the AsmMatcher isn't always applicable to the AsmOperand.

This is because the code will only update the diagnostic if it is more specific than the previous diagnostic. However, when having validated operands and 'moved on' to a next operand (for some instruction/alias for which all previous operands are valid), if the diagnostic is InvalidOperand, than that should be set as the diagnostic, not the more specific message about a previous operand for some other instruction/alias candidate.

Diff Detail

Repository
rL LLVM

Event Timeline

sdesmalen created this revision.Nov 14 2017, 2:50 AM
rengolin edited edge metadata.Nov 18 2017, 12:27 PM

@olista01 is working in this area...

olista01 edited edge metadata.Nov 21 2017, 3:30 AM

It's a shame that we have to fall back the the general "invalid operand for instruction" in all of these cases, but I don't see any easy fix for that, and it's obviously better than emitting the wrong diagnostic.

Hopefully porting the multiple-near-misses diagnostics to AArch64 will allow us to improve the diagnostics in cases like this in the future.

LGTM

olista01 accepted this revision.Nov 21 2017, 3:30 AM
This revision is now accepted and ready to land.Nov 21 2017, 3:30 AM
sdesmalen closed this revision.Nov 21 2017, 4:26 AM
sdesmalen reopened this revision.Nov 29 2017, 8:15 AM

I had to revert this patch (r318759) due to a 'make check-all failure' on a Windows buildbot that I cannot seem to reproduce.

http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/6292

I tried reproducing it on Linux and on Windows (Server 2012, built with Microsoft Visual Studio 14.0), for both Release, Debug and RelWithDebInfo builds, without success.
I have also tried things like reordering the records in the matcher table as I am aware that the way the assembler matches instructions is not always deterministic (although this patch should improve that).

Without more details from that specific buildbot builder (like output from llvm-mc) I am out of ideas on how to debug this, so any suggestions are greatly appreciated.

This revision is now accepted and ready to land.Nov 29 2017, 8:15 AM
This revision was automatically updated to reflect the committed changes.