[WebAssembly] Fix rethrow's argument type

[aheejin]

Summary

According to the changed spec, rethrow now takes not an i32 immediate
but an except_ref argument. This patch deletes the old i32imm argument,
and does not add the except_ref argument because the argument will be
added after instruction selection.

[dschuff]

Why do we need to omit the argument here? I'm not sure I understand what you mean by adding it after isel. Can/should a pass add an argument that doesn't match the format in the td?

[aheejin]

Yeah, I agree this is ugly.. but don't know how to get around it. The argument of rethrow is an except_ref value, which cannot be represented in LLVM IR. So when lowering rethrow intrinsic into rethrow instruction, if we want to add the except_ref argument to the td file, we need a dummy argument, something like nullref, to insert. Before, rethrow used to take an integer (relative depth), so we initialized the argument with 0 when lowering it, as follows:

def : Pat<(int_wasm_rethrow), (RETHROW 0)>;

But now, we don't have a zero-like value in except_ref type, like 0 above, to initialize the lowered instruction with. That's the reason I omitted the argument in the td file. Do you think we can make a zero-like dummy value for this purpose?

I think we can add arguments that are not in the td file. For example, in the td file, br and br_if takes bb_op, but CFGStackify deletes all of them and adds an immediate (relative depth) instead.

[dschuff]

I forget what the spec says right now, but I think we will need a null value for except_ref in the spec anyway, because locals are implicitly initialized to their respective 0 values. So we'd need a corresponding 0 value for ref types. So I think it would be fine to have a nullref type in LLVM too.

[aheejin]

I did a quick search on existing td files but don't know how to add a null value for a custom type. Do you know how?

BTW Rossberg said they are NOT going to add nullref to except_ref or anyref type. (WebAssembly/exception-handling#55) I still think it is worthwhile to add it to LLVM backend for the instruction selection though.

[aheejin]

This comment was removed by aheejin.

[dschuff]

Hm, another option could be undef here. I don't know if that would be any easier though.

[aheejin]

Turns out it's not even a nullref or tablegen problem. (I mean, for rethrow instruction selection here. We certainly need nullref in wasm.)

What does it mean that we have a nullref value? Every MachineOperand should either be a register or an immediate. except_ref value cannot be an immediate, so it should be in a register.

Making a fake physical register, using it as an operand to rethrow in instruction selection, and registering it as a function live-in does not work, because PrepareForLivenessIntervals pass, which creates IMPLICIT_DEF for reg uses without defs, runs after ReplacePhysRegs pass. So we wouldn't even be able to figure out that was once a live-in physical register.

So we somehow have to put nullref, or whatever value, in a register ourselves. To do that, there should be a way to create it. At this point what we need is not even a nullref, because it is now actually irrelevant. The only thing that's relevant is how we create whatever value that is. That is dependent on where we put rethrow instruction. If we put that in a library function, that means ARGUMENT_EXCEPT_REF instruction will take care of it. If we put that with catch instruction, that catch will take care of it, but in this case rethrow instruction without a catch instruction will be invalid. For the prototype, I was not gonna put rethrow within a library function because that's easier and we wouldn't need to figure out how to pass except_ref as arguments. So, in this case, to land this, we need instruction selection patch that creates this value as well. But I marked the isel patch (D44090) as "Changes Planned" for now because we are likely to change that patch if we use WinEH.

So, in conclusion, it looks like we should put off landing this as well.

[dschuff]

We don't actually create uses without reaching defs though, do we? Isn't it true that by the time we run PrepareForLiveIntervals, we've lowered the catch (and the catch will create a def of the phys reg that reaches the rethrow?)

[aheejin]

True, but the catch lowering is done in D44090 which is punted because of winEH.

[dschuff]

Sure, but once everything is in, then we shouldn't have the problem that we get undefs created. So we coudl just roll this into whatever CL adds the defs.

[aheejin]

Yeah that makes sense. Closing this then.

lib/Target/WebAssembly/WebAssemblyInstrControl.td

 // Throwing an exception: throw / rethrow
 let isTerminator = 1, hasCtrlDep = 1, isBarrier = 1 in {
 def THROW_I32 : I<(outs), (ins i32imm:$tag, I32:$val),
                   [(int_wasm_throw imm:$tag, I32:$val)], "throw   \t$tag, $val",
                   0x08>;
 def THROW_I64 : I<(outs), (ins i32imm:$tag, I64:$val),
                   [(int_wasm_throw imm:$tag, I64:$val)], "throw   \t$tag, $val",
                   0x08>;
-def RETHROW : I<(outs), (ins i32imm:$rel_depth), [], "rethrow \t$rel_depth",
-                0x09>;
+// rethrow instruction takes an except_ref type argument, but it is omitted here
+// because it will be added after instruction selection.
+def RETHROW : I<(outs), (ins), [(int_wasm_rethrow)], "rethrow", 0x09>;
 } // isTerminator = 1, hasCtrlDep = 1, isBarrier = 1
 
 // Region within which an exception is caught: try / end_try
 let Uses = [VALUE_STACK], Defs = [VALUE_STACK] in {
 def TRY     : I<(outs), (ins Signature:$sig), [], "try     \t$sig", 0x06>;
 def END_TRY : I<(outs), (ins), [], "end_try", 0x0b>;
 } // Uses = [VALUE_STACK], Defs = [VALUE_STACK]
 
 } // Defs = [ARGUMENTS]
-
-// rethrow takes a relative depth as an argument, for which currently only 0 is
-// possible for C++. Once other languages need depths other than 0, depths will
-// be computed in CFGStackify.
-def : Pat<(int_wasm_rethrow), (RETHROW 0)>;

test/CodeGen/WebAssembly/exception.ll

 ; CHECK-NEXT: i32.const $push0=, 0
 ; CHECK-NEXT: throw 0, $pop0
 define void @throw() {
   call void @llvm.wasm.throw(i32 0, i8* null)
   ret void
 }
 
 ; CHECK-LABEL: rethrow:
-; CHECK-NEXT: rethrow 0
+; CHECK-NEXT: rethrow
 define void @rethrow() {
   call void @llvm.wasm.rethrow()
   ret void
 }