[WebAssembly] Implement ref.is_null MC layer support and codegen

[asb]

Summary

Note: an earlier version of this patch was authored by @pmatos.

Custom type-checking is used to workaround the fact that separate variants of the instruction are defined for externref and funcref.

[pmatos]

A couple of weeks ago @asb pointed out we were missing an intrinsic for ref.is_null, so I got the work started. There's an issue with the polymorphic type for ref types, but I have seen this before and hopefully will get it fixed properly this time.

[asb]

Commandeered, completed implementation, and updated summary to explain current implementation approach.

[tlively]

LGTM! (with single comment)

llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp

325

How complicated would this be to fix? Is it worth fixing now?

[aheejin]

llvm/lib/Target/WebAssembly/WebAssemblyInstrRef.td

30–32

Why should the stack version and the non-stack version separately? Doesn't I provide a template to define both at the same time? Maybe I'm not familiar with the context.

Also why is this isCodeGenOnly?

[sbc100]

llvm/test/CodeGen/WebAssembly/ref-null.ll

48

Is there some reason why there are no expectations in the body here? (and above?

[asb]

Address review comments:

• Move to using update_llc_test_checks.py for ref-null.ll (and therefore fix missing CHECK lines)
• Add tighter error checking for ref.is_null in the wasm type checker

llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp

325

I'd initially skipped it due to planning to return to representation of reftypes in a bigger way. But you're right, it's not hard to provide a better message right now and I've done so by introducing a popRefType helper.

llvm/lib/Target/WebAssembly/WebAssemblyInstrRef.td

30–32

It does, but due to the way representation of reftypes is currently handled we want two register versions of the instruction (one for externref and one for funcref), but only one for the stack-based version (so ref.is_null can be printed/parsed in assembly). Marking the register instructions as isCodeGenOnly is consistent with what the I multiclass does (and correct, as the register instructions are really pseudo-instructions to bridge the gap between LLVM being register based on wasm being stack-based).

As I note in the patch summary I don't think this necessarily the globally optimum way of handling reftypes, but I want to save doing more invasive changes / refactoring until we've properly planned out handling of reftypes looking ahead to the typed function ref and GC proposals.

llvm/test/CodeGen/WebAssembly/ref-null.ll

48

That's an embarrassing oversight - thanks for catching!

I've updated this test case to use update_llc_test_checks.py (which it seems isn't compatible with -asm-verbose=false).

[aheejin]

llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h

83

Can this be just isRefType?

llvm/lib/Target/WebAssembly/WebAssemblyInstrRef.td

30–32

Why should there be only one stack variant? I think two different instructions can have the same name and opcode so they can be parsed and emitted fine?

[asb]

Update based on review comments.

llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h

83

Good point, changed.

llvm/lib/Target/WebAssembly/WebAssemblyInstrRef.td

30–32

Hmm, you're right. I'd thought this approach would be cleaner, but as it requires custom type-checking anyway that's not really the case. I've moved to just allowing the overlapping definitions.

Thanks!

[aheejin]

Thanks! I have one more question but don't want to hold this back with nitpicky things. (The review turnaround time due to time zone doesn't help either)

llvm/include/llvm/IR/IntrinsicsWebAssembly.td

34–37

Why do we need to specify names explicitly? Aren't these automatically named like these anyway?

[asb]

llvm/include/llvm/IR/IntrinsicsWebAssembly.td

34–37

The default naming would call these 'llvm.wasm.ref.is.null.foo', and I think aligning to the is_null underlying instruction name is more intuitive.

[asb]

In D123484#3509766, @aheejin wrote:

Thanks! I have one more question but don't want to hold this back with nitpicky things. (The review turnaround time due to time zone doesn't help either)

Thanks for such helpful and responsive reviews! I've left a comment about why I'm overriding the default naming (basically is.null vs is_null).

llvm/include/llvm/IR/IntrinsicsWebAssembly.td

def int_wasm_memory_grow : Intrinsic<[llvm_anyint_ty],
                                      [llvm_i32_ty, LLVMMatchType<0>],
                                      []>;
 
 //===----------------------------------------------------------------------===//
 // ref.null intrinsics
 //===----------------------------------------------------------------------===//
 def int_wasm_ref_null_extern : Intrinsic<[llvm_externref_ty], [], [IntrNoMem]>;
 def int_wasm_ref_null_func : Intrinsic<[llvm_funcref_ty], [], [IntrNoMem]>;
+def int_wasm_ref_is_null_extern : Intrinsic<[llvm_i32_ty], [llvm_externref_ty],
+                                            [IntrNoMem], "llvm.wasm.ref.is_null.extern">;
+def int_wasm_ref_is_null_func : Intrinsic<[llvm_i32_ty], [llvm_funcref_ty],
+                                          [IntrNoMem], "llvm.wasm.ref.is_null.func">;
 
 //===----------------------------------------------------------------------===//
 // Table intrinsics
 //===----------------------------------------------------------------------===//
 // Query the current table size, and increase the current table size.
 def int_wasm_table_size : Intrinsic<[llvm_i32_ty],
                                     [llvm_table_ty],
                                     [IntrReadMem]>;

llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.h

class WebAssemblyAsmTypeCheck final {
   wasm::WasmSignature LastSig;
   bool TypeErrorThisFunction = false;
   bool Unreachable = false;
   bool is64;
 
   void dumpTypeStack(Twine Msg);
   bool typeError(SMLoc ErrorLoc, const Twine &Msg);
   bool popType(SMLoc ErrorLoc, Optional<wasm::ValType> EVT);
+  bool popRefType(SMLoc ErrorLoc);
   bool getLocal(SMLoc ErrorLoc, const MCInst &Inst, wasm::ValType &Type);
   bool checkEnd(SMLoc ErrorLoc, bool PopVals = false);
   bool checkSig(SMLoc ErrorLoc, const wasm::WasmSignature &Sig);
   bool getSymRef(SMLoc ErrorLoc, const MCInst &Inst,
                  const MCSymbolRefExpr *&SymRef);
   bool getGlobal(SMLoc ErrorLoc, const MCInst &Inst, wasm::ValType &Type);
   bool getTable(SMLoc ErrorLoc, const MCInst &Inst, wasm::ValType &Type);
 

llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp

if (EVT.hasValue() && EVT.getValue() != PVT) {
     return typeError(
         ErrorLoc, StringRef("popped ") + WebAssembly::typeToString(PVT) +
                                     ", expected " +
                                     WebAssembly::typeToString(EVT.getValue()));
   }
   return false;
 }
 
+bool WebAssemblyAsmTypeCheck::popRefType(SMLoc ErrorLoc) {
+  if (Stack.empty()) {
+    return typeError(ErrorLoc, StringRef("empty stack while popping reftype"));
+  }
+  auto PVT = Stack.pop_back_val();
+  if (!WebAssembly::isRefType(PVT)) {
+    return typeError(ErrorLoc, StringRef("popped ") +
+                                   WebAssembly::typeToString(PVT) +
+                                   ", expected reftype");
+  }
+  return false;
+}
+
 bool WebAssemblyAsmTypeCheck::getLocal(SMLoc ErrorLoc, const MCInst &Inst,
                                        wasm::ValType &Type) {
   auto Local = static_cast<size_t>(Inst.getOperand(0).getImm());
   if (Local >= LocalTypes.size())
     return typeError(ErrorLoc, StringRef("no local type specified for index ") +
                           std::to_string(Local));
   Type = LocalTypes[Local];
   return false;
if (!Sig || WasmSym->getType() != wasm::WASM_SYMBOL_TYPE_TAG)
       return typeError(Operands[1]->getStartLoc(), StringRef("symbol ") +
                                                        WasmSym->getName() +
                                                        " missing .tagtype");
     // catch instruction pushes values whose types are specified in the tag's
     // "params" part
     Stack.insert(Stack.end(), Sig->Params.begin(), Sig->Params.end());
   } else if (Name == "unreachable") {
     Unreachable = true;
+  } else if (Name == "ref.is_null") {
+    if (popRefType(ErrorLoc))
+      return true;
+    Stack.push_back(wasm::ValType::I32);
   } else {
     // The current instruction is a stack instruction which doesn't have
     // explicit operands that indicate push/pop types, so we get those from
     // the register version of the same instruction.
     auto RegOpc = WebAssembly::getRegisterOpcode(Opc);
     assert(RegOpc != -1 && "Failed to get register version of MC instruction");
     const auto &II = MII.get(RegOpc);
     // First pop all the uses off the stack and check them.

llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h

inline bool isExternrefType(const Type *Ty) {
   return isa<PointerType>(Ty) &&
          Ty->getPointerAddressSpace() ==
              WasmAddressSpace::WASM_ADDRESS_SPACE_EXTERNREF;
 }
 inline bool isRefType(const Type *Ty) {
   return isFuncrefType(Ty) || isExternrefType(Ty);
 }
 
+inline bool isRefType(wasm::ValType Type) {
+  return Type == wasm::ValType::EXTERNREF || Type == wasm::ValType::FUNCREF;
+}
+
 // Convert StringRef to ValType / HealType / BlockType
 
 Optional<wasm::ValType> parseType(StringRef Type);
 BlockType parseBlockType(StringRef Type);
 MVT parseMVT(StringRef Type);
 
 // Convert ValType or a list/signature of ValTypes to a string.
 

llvm/lib/Target/WebAssembly/WebAssemblyInstrRef.td

defm REF_NULL_#rc : I<(outs rc:$dst), (ins),
                       Requires<[HasReferenceTypes]>;
   defm SELECT_#rc: I<(outs rc:$dst), (ins rc:$lhs, rc:$rhs, I32:$cond),
                      (outs), (ins),
                      [(set rc:$dst,
                        (select I32:$cond, rc:$lhs, rc:$rhs))],
                      vt#".select\t$dst, $lhs, $rhs, $cond",
                      vt#".select", 0x1b>,
                    Requires<[HasReferenceTypes]>;
+  defm REF_IS_NULL_#rc
+      : I<(outs I32:$dst), (ins rc:$ref), (outs), (ins),
+          [(set I32:$dst, (!cast<Intrinsic>("int_wasm_ref_is_null_" # ht) rc:$ref))],
+          "ref.is_null\t$ref",
+          "ref.is_null", 0xd1>,
+        Requires<[HasReferenceTypes]>;
 }
 
 defm "" : REF_I<FUNCREF, funcref, "func">;
 defm "" : REF_I<EXTERNREF, externref, "extern">;
 
 foreach rc = [FUNCREF, EXTERNREF] in {
 def : Pat<(select (i32 (setne I32:$cond, 0)), rc:$lhs, rc:$rhs),
           (!cast<Instruction>("SELECT_"#rc) rc:$lhs, rc:$rhs, I32:$cond)>;
 def : Pat<(select (i32 (seteq I32:$cond, 0)), rc:$lhs, rc:$rhs),
           (!cast<Instruction>("SELECT_"#rc) rc:$rhs, rc:$lhs, I32:$cond)>;
 }

llvm/test/CodeGen/WebAssembly/ref-null.ll

-; RUN: llc --mtriple=wasm32-unknown-unknown -asm-verbose=false -mattr=+reference-types < %s | FileCheck %s
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc --mtriple=wasm32-unknown-unknown -mattr=+reference-types < %s | FileCheck %s
 
 %extern = type opaque
 %externref = type %extern addrspace(10)* ;; addrspace 10 is nonintegral
 %funcref = type i8 addrspace(20)*      ;; addrspace 20 is nonintegral
 
 declare %externref @llvm.wasm.ref.null.extern() nounwind
 declare %funcref @llvm.wasm.ref.null.func() nounwind
+declare i32 @llvm.wasm.ref.is_null.extern(%externref) nounwind
+declare i32 @llvm.wasm.ref.is_null.func(%funcref) nounwind
 
 define %externref @get_null_extern() {
 ; CHECK-LABEL: get_null_extern:
-; CHECK-NEXT:  .functype       get_null_extern () -> (externref)
-; CHECK-NEXT:  ref.null_extern
-; CHECK-NEXT:  end_function
+; CHECK:         .functype get_null_extern () -> (externref)
+; CHECK-NEXT:  # %bb.0:
+; CHECK-NEXT:    ref.null_extern
+; CHECK-NEXT:    # fallthrough-return
   %null = call %externref @llvm.wasm.ref.null.extern()
   ret %externref %null
 }
 
 define %funcref @get_null_func() {
 ; CHECK-LABEL: get_null_func:
-; CHECK-NEXT:  .functype       get_null_func () -> (funcref)
-; CHECK-NEXT:  ref.null_func
-; CHECK-NEXT:  end_function
+; CHECK:         .functype get_null_func () -> (funcref)
+; CHECK-NEXT:  # %bb.0:
+; CHECK-NEXT:    ref.null_func
+; CHECK-NEXT:    # fallthrough-return
   %null = call %funcref @llvm.wasm.ref.null.func()
   ret %funcref %null
 }
+
+define i32 @ref_is_null_extern(%externref %eref) {
+; CHECK-LABEL: ref_is_null_extern:
+; CHECK:         .functype ref_is_null_extern (externref) -> (i32)
+; CHECK-NEXT:  # %bb.0:
+; CHECK-NEXT:    ref.null_extern
+; CHECK-NEXT:    ref.is_null
+; CHECK-NEXT:    local.get 0
+; CHECK-NEXT:    ref.is_null
+; CHECK-NEXT:    i32.add
+; CHECK-NEXT:    # fallthrough-return
+  %null = call %externref @llvm.wasm.ref.null.extern()
+  %is_null = call i32 @llvm.wasm.ref.is_null.extern(%externref %null)
+  %arg_is_null = call i32 @llvm.wasm.ref.is_null.extern(%externref %eref)
+  %res = add i32 %is_null, %arg_is_null
+  ret i32 %res
+}
+
+define i32 @ref_is_null_func(%funcref %fref) {
+; CHECK-LABEL: ref_is_null_func:
+; CHECK:         .functype ref_is_null_func (funcref) -> (i32)
+; CHECK-NEXT:  # %bb.0:
+; CHECK-NEXT:    ref.null_func
+; CHECK-NEXT:    ref.is_null
+; CHECK-NEXT:    local.get 0
+; CHECK-NEXT:    ref.is_null
+; CHECK-NEXT:    i32.add
+; CHECK-NEXT:    # fallthrough-return
+  %null = call %funcref @llvm.wasm.ref.null.func()
+  %is_null = call i32 @llvm.wasm.ref.is_null.func(%funcref %null)
+  %arg_is_null = call i32 @llvm.wasm.ref.is_null.func(%funcref %fref)
+  %res = add i32 %is_null, %arg_is_null
+  ret i32 %res
+}

llvm/test/MC/WebAssembly/reference-types.s

 # RUN: llvm-mc -show-encoding -triple=wasm32-unknown-unknown -mattr=+reference-types < %s | FileCheck %s
 # RUN: llvm-mc -show-encoding -triple=wasm64-unknown-unknown -mattr=+reference-types < %s | FileCheck %s
 
+# CHECK-LABEL:ref_is_null:
+# CHECK: ref.is_null     # encoding: [0xd1]
+ref_is_null:
+  .functype ref_is_null () -> (i32, i32)
+  ref.null_extern
+  ref.is_null
+  ref.null_func
+  ref.is_null
+  end_function
+
 # CHECK-LABEL: ref_null_test:
 # CHECK: ref.null_func   # encoding: [0xd0,0x70]
 # CHECK: ref.null_extern # encoding: [0xd0,0x6f]
 ref_null_test:
   .functype ref_null_test () -> ()
   ref.null_func
   drop
   ref.null_extern

llvm/test/MC/WebAssembly/type-checker-errors.s

catch_superfluous_value_at_end:
   .tagtype tag_i32 i32
   try
   catch tag_i32
   end_try
 # FIXME: Superfluous value should be caught at end_try?
 # CHECK: :[[@LINE+1]]:3: error: 1 superfluous return values
   end_function
 
+ref_is_null_empty_stack_while_popping:
+  .functype ref_is_null_empty_stack_while_popping () -> ()
+# CHECK: [[@LINE+1]]:3: error: empty stack while popping reftype
+  ref.is_null
+  end_function
+
+ref_is_null_type_mismatch:
+  .functype ref_is_null_type_mismatch () -> ()
+  i32.const 1
+# CHECK: [[@LINE+1]]:3: error: popped i32, expected reftype
+  ref.is_null
+  end_function
+
+ref_is_null_pushes_i32:
+  .functype ref_is_null_pushes_i32 () -> (i64)
+  ref.null_func
+  ref.is_null
+# CHECK: :[[@LINE+1]]:3: error: popped i32, expected i64
+  end_function
+
 # For the other instructions, the type checker checks vs the operands in the
 # instruction definition. Perform some simple checks for these rather than
 # exhaustively testing all instructions.
 
 other_insn_test_1:
   .functype other_insn_test_1 () -> ()
 # CHECK: [[@LINE+1]]:3: error: empty stack while popping i32
   i32.add