Remove __builtin_wasm_rethrow builtin

[aheejin]

Summary

Remove __builtin_wasm_rethrow builtin. I thought it was required to implement
__cxa_rethrow function in libcxxabi, but it turned out it will be using
__builtin_wasm_throw instead.

[dschuff]

So, it just occurred to me; we don't have 2-phase unwinding, which is useful to get good stack traces for debugging (because if you have unhandled exceptions you can abort before the second phase, rather than after the stack has been unwound). If the VM has an exception object from the original throw, it should have a stack trace attached to it. If we rethrow rather than throwing anew each time, then it will be preserved. So could we try to use rethrow instead of throw where possible? If we think it even might be feasible, it might be worth leaving rethrow in for now.

[sunfish]

It may be worth considering keeping this builtin, even if there's no immediate use for it. If someone ever does need to use this instruction, such as to implement an alternate exception-handling strategy, a builtin would mean they wouldn't have to fall back to inline asm.

[aheejin]

Hmm. I somehow thought it's not possible to implement __cxa_rethrow using rethrow instruction, but to think about it, it might be actually possible. I guess I can't restore them within this CL and should open another one...?

[aheejin]

@dschuff I can restore it, and maybe I should, but I don't know how it would make a difference in preserving stack traces. Even if I will change __cxa_rethrow's implementation to use rethrow, only thing that changes is VM will get the argument (thrown object) from a separate storage reserved for rethrow rather than get it from the top of Wasm value stack. __cxa_rethrow does not reallocate exception object no matter which instruction (throw or rethrow) it uses.

[dschuff]

Right, I was actually referring to the VM's internal tracking of exception stacks. I'm assuming it will use its usual mechanism for JS exceptions, which does keep a stack trace that you can access from JS code. Rethrowing rather than a new throw might keep that one intact. It doesn't help with the C++ state, but it's still better than nothing.

include/clang/Basic/BuiltinsWebAssembly.def

 
 // Note that current_memory is not "c" (readnone) because it must be sequenced
 // with respect to grow_memory calls.
 BUILTIN(__builtin_wasm_current_memory, "z", "n")
 BUILTIN(__builtin_wasm_grow_memory, "zz", "n")
 
 // Exception handling builtins.
 BUILTIN(__builtin_wasm_throw, "vUiv*", "r")
-BUILTIN(__builtin_wasm_rethrow, "v", "r")
 
 #undef BUILTIN

lib/CodeGen/CGBuiltin.cpp

case WebAssembly::BI__builtin_wasm_grow_memory: {
     return Builder.CreateCall(Callee, X);
   }
   case WebAssembly::BI__builtin_wasm_throw: {
     Value *Tag = EmitScalarExpr(E->getArg(0));
     Value *Obj = EmitScalarExpr(E->getArg(1));
     Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_throw);
     return Builder.CreateCall(Callee, {Tag, Obj});
   }
-  case WebAssembly::BI__builtin_wasm_rethrow: {
-    Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_rethrow);
-    return Builder.CreateCall(Callee);
-  }
 
   default:
     return nullptr;
   }
 }

test/CodeGen/builtins-wasm.c

 // WEBASSEMBLY64: call i64 @llvm.wasm.grow.memory.i64(i64 %{{.*}})
 }
 
 void f3(unsigned int tag, void *obj) {
   return __builtin_wasm_throw(tag, obj);
 // WEBASSEMBLY32: call void @llvm.wasm.throw(i32 %{{.*}}, i8* %{{.*}})
 // WEBASSEMBLY64: call void @llvm.wasm.throw(i32 %{{.*}}, i8* %{{.*}})
 }
-
-void f4() {
-  return __builtin_wasm_rethrow();
-// WEBASSEMBLY32: call void @llvm.wasm.rethrow()
-// WEBASSEMBLY64: call void @llvm.wasm.rethrow()
-}