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

[sdesmalen]

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.

[rengolin]

@olista01 is working in this area...

[olista01]

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

[sdesmalen]

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.

llvm/trunk/lib/Target/AArch64/SVEInstrFormats.td

 //=-- SVEInstrFormats.td -  AArch64 SVE Instruction classes -*- tablegen -*--=//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // AArch64 Scalable Vector Extension (SVE) Instruction Class Definitions.
 //
 //===----------------------------------------------------------------------===//
 
 //===----------------------------------------------------------------------===//
-// SVE Integer Arithmetic - Unpredicated Group.
+// SVE Integer Arithmetic -  Unpredicated Group.
 //===----------------------------------------------------------------------===//
 
 class sve_int_bin_cons_arit_0<bits<2> sz8_64, bits<3> opc, string asm,
                               ZPRRegOp zprty>
 : I<(outs zprty:$Zd), (ins zprty:$Zn, zprty:$Zm),
   asm, "\t$Zd, $Zn, $Zm",
   "", []>, Sched<[]> {
   bits<5> Zd;

llvm/trunk/test/MC/AArch64/arm64-diags.s

 ; RUN: not llvm-mc -triple arm64-apple-darwin -show-encoding < %s 2> %t | FileCheck %s
 ; RUN: FileCheck --check-prefix=CHECK-ERRORS < %t %s
 
 foo:
 
 ; The first should encode as an expression. The second should error expecting
 ; a register.
   ldr x3, (foo + 4)
   ldr x3, [foo + 4]
 ; CHECK:  ldr x3, foo+4               ; encoding: [0bAAA00011,A,A,0x58]
 ; CHECK:                              ;   fixup A - offset: 0, value: foo+4, kind: fixup_aarch64_ldr_pcrel_imm19
-; CHECK-ERRORS: error: expected label or encodable integer pc offset
+; CHECK-ERRORS: error: invalid operand for instruction
 
 ; The last argument should be flagged as an error.  rdar://9576009
   ld4.8b  {v0, v1, v2, v3}, [x0], #33
 ; CHECK-ERRORS: error: invalid operand for instruction
 ; CHECK-ERRORS: ld4.8b  {v0, v1, v2, v3}, [x0], #33
 
 
         ldr x0, [x0, #804]

 ; CHECK-ERRORS:         ldr x0, [x0, #804]
 ; CHECK-ERRORS:                 ^
 ; CHECK-ERRORS: error: index must be an integer in range [-256, 255].
 ; CHECK-ERRORS:         ldr w0, [x0, #802]
 ; CHECK-ERRORS:                 ^
 ; CHECK-ERRORS: error: index must be an integer in range [-256, 255].
 ; CHECK-ERRORS:         ldr x0, [x0, #804]!
 ; CHECK-ERRORS:                 ^
-; CHECK-ERRORS: error: expected label or encodable integer pc offset
+; CHECK-ERRORS: error: invalid operand for instruction
 ; CHECK-ERRORS:         ldr w0, [w0, #301]!
 ; CHECK-ERRORS:                  ^
 ; CHECK-ERRORS: error: index must be an integer in range [-256, 255].
 ; CHECK-ERRORS:         ldr x0, [x0], #804
 ; CHECK-ERRORS:                       ^
-; CHECK-ERRORS: error: expected label or encodable integer pc offset
+; CHECK-ERRORS: error: invalid operand for instruction
 ; CHECK-ERRORS:         ldr w0, [w0], #301
 ; CHECK-ERRORS:                  ^
 ; CHECK-ERRORS: error: index must be a multiple of 4 in range [-256, 252].
 ; CHECK-ERRORS:         ldp w3, w4, [x5, #11]!
 ; CHECK-ERRORS:                     ^
 ; CHECK-ERRORS: error: index must be a multiple of 8 in range [-512, 504].
 ; CHECK-ERRORS:         ldp x3, x4, [x5, #12]!
 ; CHECK-ERRORS:                     ^

 ; And check that we do the same for non-NEON instructions.
 
   b.ne
   b.eq 0, 0
 
 ; CHECK-ERRORS: error: too few operands for instruction
 ; CHECK-ERRORS:   b.ne
 ; CHECK-ERRORS:   ^
-; CHECK-ERRORS: error: expected label or encodable integer pc offset
+; CHECK-ERRORS: error: invalid operand for instruction
 ; CHECK-ERRORS:   b.eq 0, 0
 ; CHECK-ERRORS:           ^
 
 ; Check that we give the proper "too few operands" diagnostic instead of
 ; asserting.
 
   ldr
 
 ; CHECK-ERRORS: error: too few operands for instruction
 ; CHECK-ERRORS:   ldr
 ; CHECK-ERRORS:   ^

llvm/trunk/test/MC/AArch64/basic-a64-diagnostics.s

 // CHECK-ERROR-NEXT:         prfum pstl3strm, [xzr]
 // CHECK-ERROR-NEXT:                           ^
 
 //------------------------------------------------------------------------------
 // Load-store register (immediate post-indexed)
 //------------------------------------------------------------------------------
         ldr x3, [x4, #25], #0
         ldr x4, [x9, #0], #4
-// CHECK-ERROR-AARCH64: error: {{expected symbolic reference or integer|index must be a multiple of 8}} in range [0, 32760]
-// CHECK-ERROR-ARM64: error: expected label or encodable integer pc offset
+// CHECK-ERROR-AARCH64: error: invalid operand for instruction
+// CHECK-ERROR-ARM64: error: invalid operand for instruction
 // CHECK-ERROR-NEXT:         ldr x3, [x4, #25], #0
 // CHECK-ERROR-NEXT:                 ^
 // CHECK-ERROR-AARCH64-NEXT: error: expected label or encodable integer pc offset
 // CHECK-ERROR-AARCH64-NEXT:         ldr x4, [x9, #0], #4
 // CHECK-ERROR-AARCH64-NEXT:                           ^
 
         strb w1, [x19], #256
         strb w9, [sp], #-257

 // CHECK-ERROR-NEXT:         ldrb w9, [sp, #-257]!
 // CHECK-ERROR-NEXT:                  ^
 // CHECK-ERROR-NEXT: error: invalid operand for instruction
 // CHECK-ERROR-NEXT:         ldrh w1, [x19, #256]!
 // CHECK-ERROR-NEXT:                             ^
 // CHECK-ERROR-NEXT: error: {{expected|index must be an}} integer in range [-256, 255]
 // CHECK-ERROR-NEXT:         ldrh w9, [sp, #-257]!
 // CHECK-ERROR-NEXT:                  ^
-// CHECK-ERROR-NEXT: error: expected label or encodable integer pc offset
+// CHECK-ERROR-NEXT: error: invalid operand for instruction
 // CHECK-ERROR-NEXT:         ldr w1, [x19, #256]!
 // CHECK-ERROR-NEXT:                            ^
 // CHECK-ERROR-NEXT: error: {{expected|index must be an}} integer in range [-256, 255]
 // CHECK-ERROR-NEXT:         ldr w9, [sp, #-257]!
 // CHECK-ERROR-NEXT:                 ^
 
         ldrsb x2, [x3, #256]!
         ldrsb x22, [x13, #-257]!
         ldrsh x2, [x3, #256]!
         ldrsh x22, [x13, #-257]!
         ldrsw x2, [x3, #256]!
         ldrsw x22, [x13, #-257]!
 // CHECK-ERROR: error: invalid operand for instruction
 // CHECK-ERROR-NEXT:         ldrsb x2, [x3, #256]!
 // CHECK-ERROR-NEXT:                             ^
 // CHECK-ERROR-NEXT: error: {{expected|index must be an}} integer in range [-256, 255]
 // CHECK-ERROR-NEXT:         ldrsb x22, [x13, #-257]!
 // CHECK-ERROR-NEXT:                    ^
 // CHECK-ERROR-NEXT: error: invalid operand for instruction
 // CHECK-ERROR-NEXT:         ldrsh x2, [x3, #256]!
 // CHECK-ERROR-NEXT:                             ^
 // CHECK-ERROR-NEXT: error: {{expected|index must be an}} integer in range [-256, 255]
 // CHECK-ERROR-NEXT:         ldrsh x22, [x13, #-257]!
 // CHECK-ERROR-NEXT:                    ^
-// CHECK-ERROR-NEXT: error: expected label or encodable integer pc offset
+// CHECK-ERROR-NEXT: error: invalid operand for instruction
 // CHECK-ERROR-NEXT:         ldrsw x2, [x3, #256]!
 // CHECK-ERROR-NEXT:                             ^
 // CHECK-ERROR-NEXT: error: {{expected|index must be an}} integer in range [-256, 255]
 // CHECK-ERROR-NEXT:         ldrsw x22, [x13, #-257]!
 // CHECK-ERROR-NEXT:                    ^
 
         ldrsb w2, [x3, #256]!
         ldrsb w22, [x13, #-257]!

 // CHECK-ERROR-NEXT:         ldr b3, [x13, #-257]!
 // CHECK-ERROR-NEXT:                 ^
 // CHECK-ERROR-NEXT: error: invalid operand for instruction
 // CHECK-ERROR-NEXT:         ldr h3, [x3, #256]!
 // CHECK-ERROR-NEXT:                           ^
 // CHECK-ERROR-NEXT: error: {{expected|index must be an}} integer in range [-256, 255]
 // CHECK-ERROR-NEXT:         ldr h3, [x13, #-257]!
 // CHECK-ERROR-NEXT:                 ^
-// CHECK-ERROR-NEXT: error: expected label or encodable integer pc offset
+// CHECK-ERROR-NEXT: error: invalid operand for instruction
 // CHECK-ERROR-NEXT:         ldr s3, [x3, #256]!
 // CHECK-ERROR-NEXT:                           ^
 // CHECK-ERROR-NEXT: error: {{expected|index must be an}} integer in range [-256, 255]
 // CHECK-ERROR-NEXT:         ldr s3, [x13, #-257]!
 // CHECK-ERROR-NEXT:                 ^
-// CHECK-ERROR-NEXT: error: expected label or encodable integer pc offset
+// CHECK-ERROR-NEXT: error: invalid operand for instruction
 // CHECK-ERROR-NEXT:         ldr d3, [x3, #256]!
 // CHECK-ERROR-NEXT:                           ^
 // CHECK-ERROR-NEXT: error: {{expected|index must be an}} integer in range [-256, 255]
 // CHECK-ERROR-NEXT:         ldr d3, [x13, #-257]!
 // CHECK-ERROR-NEXT:                 ^
 
 //------------------------------------------------------------------------------
 // Load/store (unprivileged)

 // CHECK-ERROR-AARCH64-NEXT:                   ^
 // CHECK-ERROR-AARCH64-NEXT: error: too few operands for instruction
 // CHECK-ERROR-AARCH64-NEXT:         str q0, [x0, #8]
 // CHECK-ERROR-AARCH64-NEXT:                 ^
 
 //// 32-bit addresses
         ldr w0, [w20]
         ldrsh x3, [wsp]
-// CHECK-ERROR: error: expected label or encodable integer pc offset
+// CHECK-ERROR: error: invalid operand for instruction
 // CHECK-ERROR-NEXT:         ldr w0, [w20]
 // CHECK-ERROR-NEXT:                  ^
 // CHECK-ERROR-NEXT: error: invalid operand for instruction
 // CHECK-ERROR-NEXT:         ldrsh x3, [wsp]
 // CHECK-ERROR-NEXT:                    ^
 
 //// Store things
         strb w0, [wsp]

 // CHECK-ERROR-AARCH64: error: Invalid immediate for instruction
 // CHECK-ERROR-ARM64: error: prefetch operand out of range, [0,31] expected
 // CHECK-ERROR-NEXT:        prfm #-1, [sp]
 // CHECK-ERROR-NEXT:             ^
 // CHECK-ERROR-AARCH64-NEXT: error: Invalid immediate for instruction
 // CHECK-ERROR-ARM64-NEXT: error: prefetch operand out of range, [0,31] expected
 // CHECK-ERROR-NEXT:        prfm #32, [sp, #8]
 // CHECK-ERROR-NEXT:             ^
-// CHECK-ERROR-NEXT: error: expected label or encodable integer pc offset
+// CHECK-ERROR-NEXT: error: invalid operand for instruction
 // CHECK-ERROR-NEXT:        prfm pldl1strm, [w3, #8]
 // CHECK-ERROR-NEXT:                         ^
 // CHECK-ERROR-AARCH64-NEXT: error: operand specifier not recognised
 // CHECK-ERROR-ARM64-NEXT: error: pre-fetch hint expected
 // CHECK-ERROR-NEXT:        prfm wibble, [sp]
 // CHECK-ERROR-NEXT:             ^
 
 //------------------------------------------------------------------------------
 // Load/store register (register offset)
 //------------------------------------------------------------------------------
 
         ldr w3, [xzr, x3]
         ldr w4, [x0, x4, lsl]
         ldr w9, [x5, x5, uxtw]
         ldr w10, [x6, x9, sxtw #2]
         ldr w11, [x7, w2, lsl #2]
         ldr w12, [x8, w1, sxtx]
-// CHECK-ERROR-NEXT: error: expected label or encodable integer pc offset
+// CHECK-ERROR-NEXT: error: invalid operand for instruction
 // CHECK-ERROR-NEXT:        ldr w3, [xzr, x3]
 // CHECK-ERROR-NEXT:                 ^
 // CHECK-ERROR-NEXT: error: expected #imm after shift specifier
 // CHECK-ERROR-NEXT:         ldr w4, [x0, x4, lsl]
 // CHECK-ERROR-NEXT:                             ^
 // CHECK-ERROR-NEXT: error: expected 'lsl' or 'sxtx' with optional shift of #0 or #2
 // CHECK-ERROR-NEXT:         ldr w9, [x5, x5, uxtw]
 // CHECK-ERROR-NEXT:                          ^

 // CHECK-ERROR-NEXT:                  ^
 
         movz x12, #:abs_g0:sym, lsl #16
         movz x12, #:abs_g0:sym, lsl #0
         movn x2, #:abs_g0:sym
         movk w3, #:abs_g0:sym
         movz x3, #:abs_g0_nc:sym
         movn x4, #:abs_g0_nc:sym
-// CHECK-ERROR: error: {{expected relocated symbol or|immediate must be an}} integer in range [0, 65535]
+// CHECK-ERROR: error: invalid operand for instruction
 // CHECK-ERROR-NEXT:         movz x12, #:abs_g0:sym, lsl #16
 // CHECK-ERROR-NEXT:                                 ^
-// CHECK-ERROR-NEXT: error: {{expected relocated symbol or|immediate must be an}} integer in range [0, 65535]
+// CHECK-ERROR:  error: invalid operand for instruction
 // CHECK-ERROR-NEXT:         movz x12, #:abs_g0:sym, lsl #0
 // CHECK-ERROR-NEXT:                                 ^
 // CHECK-ERROR-AARCH64-NEXT: error: {{expected relocated symbol or|immediate must be an}} integer in range [0, 65535]
 // CHECK-ERROR-AARCH64-NEXT:         movn x2, #:abs_g0:sym
 // CHECK-ERROR-AARCH64-NEXT:                  ^
 // CHECK-ERROR-NEXT: error: {{expected relocated symbol or|immediate must be an}} integer in range [0, 65535]
 // CHECK-ERROR-NEXT:         movk w3, #:abs_g0:sym
 // CHECK-ERROR-NEXT:                  ^

llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp

if (ReportMultipleNearMisses) {
     OS << "    }\n\n";
   } else {
     OS << "      // If this operand is broken for all of the instances of this\n";
     OS << "      // mnemonic, keep track of it so we can report loc info.\n";
     OS << "      // If we already had a match that only failed due to a\n";
     OS << "      // target predicate, that diagnostic is preferred.\n";
     OS << "      if (!HadMatchOtherThanPredicate &&\n";
     OS << "          (it == MnemonicRange.first || ErrorInfo <= ActualIdx)) {\n";
-    OS << "        ErrorInfo = ActualIdx;\n";
-    OS << "        // InvalidOperand is the default. Prefer specificity.\n";
-    OS << "        if (Diag != Match_InvalidOperand)\n";
+    OS << "        if (Diag != Match_InvalidOperand || ErrorInfo != ActualIdx)\n";
     OS << "          RetCode = Diag;\n";
+    OS << "        ErrorInfo = ActualIdx;\n";
     OS << "      }\n";
     OS << "      // Otherwise, just reject this instance of the mnemonic.\n";
     OS << "      OperandsValid = false;\n";
     OS << "      break;\n";
     OS << "    }\n\n";
   }
 
   if (ReportMultipleNearMisses)