Bugzilla

(In reply to Benjamin Bouvier [:bbouvier] from comment #22)
> > +  DebugOnly<bool> called = false;
> 
> nit: DebugOnly is not 0 bytes in non-debug builds, so we use #ifdef DEBUG
> for class members.

You're right in general but given that this is a shell testing function and that it's a MOZ_STACK_CLASS (and thus may be [SRA'd](https://llvm.org/docs/Passes.html#sroa-scalar-replacement-of-aggregates) anyhow), it didn't seem worth the visual ugliness to me.

> So we might need something like:
> 
> compileArgs->ionEnabled = IonCanCompile();
> #ifdef ENABLE_WASM_CRANELIFT
> compileArgs->craneliftEnabled = !IonCanCompile() && CraneliftCanCompile();
> // prefer Ion over Cranelift by default.
> #endif

Since IonCanCompile() will always be true when CraneliftCanCompile() is true, I think adding this code won't help.  I think the real fix is to propagate the --wasm-ion / --wasm-cranelift flags from parent to child and then have the child shell's WasmCompileAndSerialize() can use these flags to pick the right compiler.  I think that would be a follow-up, though, when we want to start testing cranelift serialization.

> This is correct because even an empty module would have e.g. length headers
> for serialized vectors, so the serialized byte vector is never empty, right?

Yep!

(In reply to Benjamin Bouvier [:bbouvier] from comment #22)
> > +  DebugOnly<bool> called = false;
> 
> nit: DebugOnly is not 0 bytes in non-debug builds, so we use #ifdef DEBUG
> for class members.

You're right in general but given that this is a shell testing function and that it's a MOZ_STACK_CLASS (and thus may be [SRA'd](https://llvm.org/docs/Passes.html#sroa-scalar-replacement-of-aggregates) anyhow), it didn't seem worth the visual ugliness to me.

> So we might need something like:
> 
> compileArgs->ionEnabled = IonCanCompile();
> #ifdef ENABLE_WASM_CRANELIFT
> compileArgs->craneliftEnabled = !IonCanCompile() && CraneliftCanCompile();
> // prefer Ion over Cranelift by default.
> #endif

Since IonCanCompile() will always be true when CraneliftCanCompile() is true, I think adding this code won't help.  I think the real fix is to propagate the --wasm-ion / --wasm-cranelift flags from parent to child so that WasmCompileAndSerialize() in the child can use these flags to pick the right compiler.  I think that would be a follow-up, though, when we want to start testing cranelift serialization.

> This is correct because even an empty module would have e.g. length headers
> for serialized vectors, so the serialized byte vector is never empty, right?

Yep!