[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

312 ↗

(On Diff #428347)

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

44

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

312 ↗

(On Diff #428347)

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

44

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 ↗

(On Diff #428581)

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 ↗

(On Diff #428581)

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 : Intrinsic<[llvm_i32_ty], [llvm_any_ty], [IntrNoMem]>;
 
 //===----------------------------------------------------------------------===//
 // 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]>;
 def int_wasm_table_copy : Intrinsic<[],
                                     [llvm_table_ty, llvm_table_ty, llvm_i32_ty, llvm_i32_ty, llvm_i32_ty],
                                     []>;

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 I32:$dst), (ins),
+                           [(set I32:$dst, (int_wasm_ref_is_null 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)>;
-}
+}
 No newline at end of file

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

 ; RUN: llc --mtriple=wasm32-unknown-unknown -asm-verbose=false -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() 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
   %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
   %null = call %funcref @llvm.wasm.ref.null.func()
   ret %funcref %null
 }
+
+define i32 @ref_is_null_eref(%extenref %eref) {
+  ; CHECK-LABEL: ref_is_null:
+  ; CHECK-NEXT: .functype      ref_is_null (externref) -> (i32)
+  %null = call %externref @llvm.wasm.ref.null.extern()
+  %is_null = call i32 @llvm.wasm.ref.is_null(%externref %null)
+  %arg_is_null = call i32 @llvm.wasm.ref.is_null(%externref %eref)
+  %res = i32 add nsw i32 %is_null, i32 %arg_is_null
+  ret i32 %res
+}
+
+define i32 @ref_is_null_fref(%funcref %fref) {
+  ; CHECK-LABEL: ref_is_null:
+  ; CHECK-NEXT: .functype      ref_is_null (externref) -> (i32)
+  %null = call %funcref @llvm.wasm.ref.null.func()
+  %is_null = call i32 @llvm.wasm.ref.is_null(%funcref %null)
+  %arg_is_null = call i32 @llvm.wasm.ref.is_null(%funcref %fref)
+  %res = i32 add nsw i32 %is_null, i32 %arg_is_null
+  ret i32 %res
+}
 No newline at end of file

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 () -> ()
+  ref.null_func
+  ref.is_null
+  drop
+  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