zig

zig copied to clipboard

This implements the majority of floating-point operations on the native webassembly backend by either calling into compiler-rt or directly emitting instructions if available. Prior to this, operations on f16 were implemented using a round trip to f32 (which is why I had to change the archive test).

Nice work! I like what you did with building the operands programmatically. I’ll do a proper review this weekend.

I disagree with using a compiler-rt function when there is a directly supported wasm operation. If the compiler-rt function is faster than the native operation, that is a problem with the wasm runtime being used. Zig is not in the business of caring about which wasm runtimes are being used. It only cares about outputting semantically correct wasm code. How did you check if it was "faster" anyway? Are you sure that there isn't a runtime in existence where the native operation performs better?

I disagree with using a compiler-rt function when there is a directly supported wasm operation.

Maybe I am misunderstanding what is being meant by "directly supported wasm operation" but as an example, there is no single wasm instruction that computes f16 addition or a logarithm on any float size. If there an instruction is available like f32 multiplication or f64 square root, the code-gen i wrote will use them instead of calling into compiler-rt functions.

If the compiler-rt function is faster than the native operation, that is a problem with the wasm runtime being used. Zig is not in the business of caring about which wasm runtimes are being used. It only cares about outputting semantically correct wasm code. How did you check if it was "faster" anyway? Are you sure that there isn't a runtime in existence where the native operation performs better?

If i recall correctly, the discussion that I had with Luukdegram was not about whether or not to use a native wasm instruction instead of compiler-rt, but instead about whether it would be beneficial to use certain wasm instructions so that we can use a different but faster compiler-rt function.

One of them was using __extendhfsf2 followed by a f64.promote_f32 instead of directly using __extendhfdf2 to convert a f16 to f64. If needed, I can elaborate on that but in short, __extendhfdf2 has a few more instructions than __extendhfsf2 than could avoided by using f64.promote_f32 which was turned out to be faster. Of course less instructions doesn't necessarily mean faster and different wasm runtimes may behave differently, but it doesn't sound like this is what you were referring to when you disagreed, right?

I disagree with using a compiler-rt function when there is a directly supported wasm operation. If the compiler-rt function is faster than the native operation, that is a problem with the wasm runtime being used. Zig is not in the business of caring about which wasm runtimes are being used. It only cares about outputting semantically correct wasm code. How did you check if it was "faster" anyway? Are you sure that there isn't a runtime in existence where the native operation performs better?

As @Techatrix mentioned, this PR always prioritizes native Wasm instructions over compiler-rt. It only falls back to a compiler-rt call when such instruction does not exist (e.g. f16->f32 uses compiler-rt, whereas f32->f64 uses a native Wasm instruction). Does this clear up any confusion? I'd like to see this merged but won't take any action unless you're satisfied with the chosen solution also.

Thank you for your patience, my break took a bit longer than originally anticipated. Great work, I'm confident in the solution that was implemented so will give this a merge.