[WebAssembly] Make disassembler always emit most canonical name.

[aardappel]

Summary

There are a few instructions that all map to the same opcode, so
when disassembling, we have to pick one. That was just the first one
before (the except_ref variant in the case of "call"), now it is the
one marked as IsCanonical in tablegen, or failing that, the shortest
name (which is typically the "canonical" one).

Also introduced a canonical "end" instruction for this purpose.

[sbc100]

I'm probably not qualified to review the tablegen stuff, but seems like a nice solution.

[tlively]

Looks great! Thanks!

[aheejin]

lib/Target/WebAssembly/WebAssemblyInstrCall.td

96 ↗

(On Diff #185158)

I don't think we indent within a let block in .td files. But I also see we do in some parts in this WebAssemblyInstrCall.td.. How about making a separate CL that disables all indentation within let blocks and making that this CL dependent on that CL? For other .td files as well.

[dschuff]

test/MC/Disassembler/WebAssembly/wasm.txt

36 ↗

(On Diff #185158)

should this comment be updated now?

[aardappel]

lib/Target/WebAssembly/WebAssemblyInstrCall.td

96 ↗

(On Diff #185158)

There appears to be a mix of these, but not indenting is more common. So for now I'll just not indent my new code.

[aardappel]

Code review fixes.

llvm/trunk/lib/Target/WebAssembly/WebAssemblyInstrCall.td

let Uses = [SP32, SP64], isCall = 1 in {
   defm "" : CALL<EXCEPT_REF, "except_ref.">;
   defm "" : SIMD_CALL<v16i8, "v128.">;
   defm "" : SIMD_CALL<v8i16, "v128.">;
   defm "" : SIMD_CALL<v4i32, "v128.">;
   defm "" : SIMD_CALL<v2i64, "v128.">;
   defm "" : SIMD_CALL<v4f32, "v128.">;
   defm "" : SIMD_CALL<v2f64, "v128.">;
 
+  let IsCanonical = 1 in {
   defm CALL_VOID : I<(outs), (ins function32_op:$callee, variable_ops),
-                     (outs), (ins function32_op:$callee),
-                     [(WebAssemblycall0 (i32 imm:$callee))],
-                     "call    \t$callee", "call\t$callee", 0x10>;
 
   let isCodeGenOnly = 1 in {
     defm PCALL_INDIRECT_VOID : I<(outs), (ins I32:$callee, variable_ops),
-                                 (outs), (ins I32:$callee),
-                                 [(WebAssemblycall0 I32:$callee)],
-                                 "PSEUDO CALL INDIRECT\t$callee",
-                                 "PSEUDO CALL INDIRECT\t$callee">;
   } // isCodeGenOnly = 1
 
   defm CALL_INDIRECT_VOID : I<(outs),
                               (ins TypeIndex:$type, i32imm:$flags,
                                 variable_ops),
                               (outs), (ins TypeIndex:$type, i32imm:$flags),
                               [],
                               "call_indirect\t", "call_indirect\t$type",
                               0x11>;
+  }
 } // Uses = [SP32,SP64], isCall = 1
 
 // Patterns for matching a direct call to a global address.
 def : Pat<(i32 (WebAssemblycall1 (WebAssemblywrapper tglobaladdr:$callee))),
           (CALL_I32 tglobaladdr:$callee)>;
 def : Pat<(i64 (WebAssemblycall1 (WebAssemblywrapper tglobaladdr:$callee))),
           (CALL_I64 tglobaladdr:$callee)>;
 def : Pat<(f32 (WebAssemblycall1 (WebAssemblywrapper tglobaladdr:$callee))),

llvm/trunk/lib/Target/WebAssembly/WebAssemblyInstrControl.td

defm IF : I<(outs), (ins Signature:$sig, I32:$cond),
             [], "if    \t$sig, $cond", "if    \t$sig", 0x04>;
 defm ELSE : NRI<(outs), (ins), [], "else", 0x05>;
 
 // END_BLOCK, END_LOOP, END_IF and END_FUNCTION are represented with the same
 // opcode in wasm.
 defm END_BLOCK : NRI<(outs), (ins), [], "end_block", 0x0b>;
 defm END_LOOP  : NRI<(outs), (ins), [], "end_loop", 0x0b>;
 defm END_IF    : NRI<(outs), (ins), [], "end_if", 0x0b>;
+// Generic instruction, for disassembler.
+let IsCanonical = 1 in {
+defm END       : NRI<(outs), (ins), [], "end", 0x0b>;
+}
 let isTerminator = 1, isBarrier = 1 in
 defm END_FUNCTION : NRI<(outs), (ins), [], "end_function", 0x0b>;
 } // Uses = [VALUE_STACK], Defs = [VALUE_STACK]
 
 multiclass RETURN<WebAssemblyRegClass vt> {
   defm RETURN_#vt : I<(outs), (ins vt:$val), (outs), (ins),
                       [(WebAssemblyreturn vt:$val)],
                       "return  \t$val", "return", 0x0f>;

llvm/trunk/lib/Target/WebAssembly/WebAssemblyInstrFormats.td

 class WebAssemblyInst<bits<32> inst, string asmstr, string stack> : StackRel,
   Instruction {
   bits<32> Inst = inst; // Instruction encoding.
   string StackBased = stack;
   string BaseName = NAME;
   let Namespace   = "WebAssembly";
   let Pattern     = [];
   let AsmString   = asmstr;
+  // When there are multiple instructions that map to the same encoding (in
+  // e.g. the disassembler use case) prefer the one where IsCanonical == 1.
+  bit IsCanonical = 0;
 }
 
 // Normal instructions. Default instantiation of a WebAssemblyInst.
 class NI<dag oops, dag iops, list<dag> pattern, string stack,
          string asmstr = "", bits<32> inst = -1>
     : WebAssemblyInst<inst, asmstr, stack> {
   dag OutOperandList = oops;
   dag InOperandList  = iops;

llvm/trunk/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td

defm "" : ConstVec<v8i16,
                    (ins vec_i16imm_op:$i0, vec_i16imm_op:$i1,
                         vec_i16imm_op:$i2, vec_i16imm_op:$i3,
                         vec_i16imm_op:$i4, vec_i16imm_op:$i5,
                         vec_i16imm_op:$i6, vec_i16imm_op:$i7),
                    (build_vector
                      ImmI16:$i0, ImmI16:$i1, ImmI16:$i2, ImmI16:$i3,
                      ImmI16:$i4, ImmI16:$i5, ImmI16:$i6, ImmI16:$i7),
                    "$i0, $i1, $i2, $i3, $i4, $i5, $i6, $i7">;
+let IsCanonical = 1 in {
 defm "" : ConstVec<v4i32,
-                   (ins vec_i32imm_op:$i0, vec_i32imm_op:$i1,
                         vec_i32imm_op:$i2, vec_i32imm_op:$i3),
-                   (build_vector (i32 imm:$i0), (i32 imm:$i1),
-                                 (i32 imm:$i2), (i32 imm:$i3)),
-                   "$i0, $i1, $i2, $i3">;
+}
 defm "" : ConstVec<v2i64,
                    (ins vec_i64imm_op:$i0, vec_i64imm_op:$i1),
                    (build_vector (i64 imm:$i0), (i64 imm:$i1)),
                    "$i0, $i1">;
 defm "" : ConstVec<v4f32,
                    (ins f32imm_op:$i0, f32imm_op:$i1,
                         f32imm_op:$i2, f32imm_op:$i3),
                    (build_vector (f32 fpimm:$i0), (f32 fpimm:$i1),

llvm/trunk/test/MC/Disassembler/WebAssembly/wasm.txt

 # CHECK: block f64
 0x02 0x7C
 
 # CHECK: call_indirect
 # $0=, 128, 0
 # FIXME: WebAssemblyInstPrinter does not print immediates.
 0x11 0x80 0x01 0x00
 
+# CHECK: call 0
+# CHECK-NOT: except_ref.call 0
+0x10 0x00
+
 # CHECK: local.get 128
 0x20 0x80 0x01
 
 # Prefix byte example:
 # CHECK: i64.trunc_sat_f64_u
 0xFC 0x07
 
-# v128.const is arbitrarily disassembled as v16i8
-# CHECK: v128.const 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15
+# CHECK: v128.const 50462976, 117835012, 185207048, 252579084
 0xFD 0x02 0x00 0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08 0x09 0x0A 0x0B 0x0C 0x0D 0x0E 0x0F
 
 # CHECK: v8x16.shuffle 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15
 0xFD 0x03 0x00 0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08 0x09 0x0A 0x0B 0x0C 0x0D 0x0E 0x0F
 
 # Check LEB128 encoding of SIMD instructions
 # CHECK: i64x2.all_true
 0xFD 0x86 0x01
 
 # Including non-canonical LEB128 encodings
 # CHECK: i64x2.any_true
 # CHECK-NOT: i64.div_u
 0xFD 0x85 0x81 0x80 0x80 0x80 0x80 0x00
 
 # Check br_table, which has its own operand type.
 # CHECK: br_table {0, 1, 2}
 0x0E 0x02 0x00 0x01 0x02
+
+# This can mean end_block/end_loop/end_if/end_function/end_try..
+# CHECK: end
+0x0B

llvm/trunk/test/MC/WebAssembly/objdump.s

test1:
     end_function
 
 
 # CHECK-LABEL: CODE:
 # CHECK:             # 2 functions in section.
 # CHECK-LABEL: test0:
 # CHECK-NEXT:        .local  f32, f64, v128, v128
 # CHECK-NEXT:       9:       20 02  local.get 2
-# CHECK-NEXT:       b:       0b     end_block
+# CHECK-NEXT:       b:       0b     end
 # CHECK-LABEL: test1:
 # CHECK-NEXT:        .local  i32, i64, except_ref
 # CHECK-NEXT:      14:       20 03  local.get 3
-# CHECK-NEXT:      16:       0b     end_block
+# CHECK-NEXT:      16:       0b     end

llvm/trunk/utils/TableGen/WebAssemblyDisassemblerEmitter.cpp

for (unsigned I = 0; I != NumberedInstructions.size(); ++I) {
     auto &CGIP = OpcodeTable[Prefix][Opc];
     // All wasm instructions have a StackBased field of type string, we only
     // want the instructions for which this is "true".
     auto StackString =
         Def.getValue("StackBased")->getValue()->getCastTo(StringRecTy::get());
     auto IsStackBased =
         StackString &&
         reinterpret_cast<const StringInit *>(StackString)->getValue() == "true";
-    if (IsStackBased && !CGIP.second) {
-      // this picks the first of many typed variants, which is
-      // currently the except_ref one, though this shouldn't matter for
-      // disassembly purposes.
-      CGIP = std::make_pair(I, &CGI);
+    if (!IsStackBased)
+      continue;
+    if (CGIP.second) {
+      // We already have an instruction for this slot, so decide which one
+      // should be the canonical one. This determines which variant gets
+      // printed in a disassembly. We want e.g. "call" not "i32.call", and
+      // "end" when we don't know if its "end_loop" or "end_block" etc.
+      auto IsCanonicalExisting = CGIP.second->TheDef->getValue("IsCanonical")
+                                     ->getValue()
+                                     ->getAsString() == "1";
+      // We already have one marked explicitly as canonical, so keep it.
+      if (IsCanonicalExisting)
+        continue;
+      auto IsCanonicalNew =
+          Def.getValue("IsCanonical")->getValue()->getAsString() == "1";
+      // If the new one is explicitly marked as canonical, take it.
+      if (!IsCanonicalNew) {
+        // Neither the existing or new instruction is canonical.
+        // Pick the one with with the shortest name as heuristic.
+        // Though ideally IsCanonical is always defined for at least one
+        // variant so this never has to apply.
+        if (CGIP.second->AsmString.size() <= CGI.AsmString.size())
+          continue;
+      }
     }
+    // Set this instruction as the one to use.
+    CGIP = std::make_pair(I, &CGI);
   }
   OS << "#include \"MCTargetDesc/WebAssemblyMCTargetDesc.h\"\n";
   OS << "\n";
   OS << "namespace llvm {\n\n";
   OS << "static constexpr int WebAssemblyInstructionTableSize = ";
   OS << WebAssemblyInstructionTableSize << ";\n\n";
   OS << "enum EntryType : uint8_t { ";
   OS << "ET_Unused, ET_Prefix, ET_Instruction };\n\n";