implement more of Alex' suggestions

Author: frank-emrich

crates/cranelift/src/func_environ.rs

@@ -91,7 +91,6 @@ wasmtime_environ::foreach_builtin_function!(declare_function_signatures);
 /// The `FuncEnvironment` implementation for use by the `ModuleEnvironment`.
 pub struct FuncEnvironment<'module_environment> {
     compiler: &'module_environment Compiler,
-    /// NOTE(frank-emrich) pub for use in crate::wasmfx::* modules
     pub(crate) isa: &'module_environment (dyn TargetIsa + 'module_environment),
     pub(crate) module: &'module_environment Module,
     pub(crate) types: &'module_environment ModuleTypesBuilder,
@@ -138,7 +137,6 @@ pub struct FuncEnvironment<'module_environment> {
     /// VMStoreContext` for this function's vmctx argument. This pointer is stored
     /// in the vmctx itself, but never changes for the lifetime of the function,
     /// so if we load it up front we can continue to use it throughout.
-    /// NOTE(frank-emrich) pub for use in stack_switching modules
     pub(crate) vmstore_context_ptr: ir::Value,
 
     /// A cached epoch deadline value, when performing epoch-based
@@ -1719,7 +1717,8 @@ impl<'module_environment> TargetEnvironment for FuncEnvironment<'module_environm
         let needs_stack_map = match wasm_ty.top() {
             WasmHeapTopType::Extern | WasmHeapTopType::Any => true,
             WasmHeapTopType::Func => false,
-            // FIXME(frank-emrich) Don't we actually need to include continuations in stack maps??
+            // TODO(#10248) Once continuations can be stored on the GC heap, we
+            // will need stack maps for continuation objects.
             WasmHeapTopType::Cont => false,
         };
         (ty, needs_stack_map)
@@ -3279,7 +3278,10 @@ impl FuncEnvironment<'_> {
         _tag_index: u32,
         _cont: ir::Value,
     ) -> WasmResult<ir::Value> {
-        todo!()
+        // TODO(#10248)
+        Err(wasmtime_environ::WasmError::Unsupported(
+            "resume.throw instruction not implemented, yet".to_string(),
+        ))
     }
 
     pub fn translate_suspend(

crates/cranelift/src/gc/enabled.rs

@@ -1068,7 +1068,10 @@ pub fn translate_ref_test(
             func_env.is_subtype(builder, actual_shared_ty, expected_shared_ty)
         }
         WasmHeapType::ConcreteCont(_) => {
-            unimplemented!("Stack switching feature not compatbile with GC, yet")
+            // TODO(#10248)
+            return Err(wasmtime_environ::WasmError::Unsupported(
+                "Stack switching feature not compatbile with GC, yet".to_string(),
+            ));
         }
     };
     builder.ins().jump(continue_block, &[result]);

crates/cranelift/src/stack_switching/control_effect.rs

@@ -8,8 +8,8 @@ use wasmtime_environ::stack_switching as stack_switching_environ;
 /// resume signal, suspension signal, and handler index into a
 /// u64 value. This instance is used at compile time. There is a runtime
 /// counterpart in `continuations/src/lib.rs`.
-/// We convert to and from u64 as follows: The 4 LSBs of the u64 are the
-/// discriminant, the 4 MSBs are the handler_index (if `Suspend`)
+/// We convert to and from u64 as follows: The low 32 bits of the u64 are the
+/// discriminant, the high 32 bits are the handler_index (if `Suspend`)
 #[derive(Clone, Copy)]
 pub struct ControlEffect(ir::Value);
 
@@ -37,7 +37,7 @@ impl ControlEffect {
     ) -> Self {
         let discriminant = builder.ins().iconst(
             I64,
-            stack_switching_environ::CONTROL_EFFECT_RESUME_DISCRIMINANT as i64,
+            i64::from(stack_switching_environ::CONTROL_EFFECT_RESUME_DISCRIMINANT),
         );
         let val = builder.ins().ishl_imm(discriminant, 32);
 
@@ -50,7 +50,7 @@ impl ControlEffect {
     ) -> Self {
         let discriminant = builder.ins().iconst(
             I64,
-            stack_switching_environ::CONTROL_EFFECT_SWITCH_DISCRIMINANT as i64,
+            i64::from(stack_switching_environ::CONTROL_EFFECT_SWITCH_DISCRIMINANT),
         );
         let val = builder.ins().ishl_imm(discriminant, 32);
 
@@ -64,7 +64,7 @@ impl ControlEffect {
     ) -> Self {
         let discriminant = builder.ins().iconst(
             I64,
-            stack_switching_environ::CONTROL_EFFECT_SUSPEND_DISCRIMINANT as i64,
+            i64::from(stack_switching_environ::CONTROL_EFFECT_SUSPEND_DISCRIMINANT),
         );
         let val = builder.ins().ishl_imm(discriminant, 32);
         let handler_index = builder.ins().uextend(I64, handler_index);