Add TrapsNeverHappen to SideEffects's API

[MaxGraey]

No description provided.

[kripken]

Thanks!

Btw, I hope perhaps you can enable this by default in AssemblyScript? In Emscripten it's too risky, as users might use clang's builtin_trap and not want that to go away (and there is little benefit after LLVM anyhow) but maybe for AS it's possible.

[MaxGraey]

Thanks!

Btw, I hope perhaps you can enable this by default in AssemblyScript? In Emscripten it's too risky, as users might use clang's builtin_trap and not want that to go away (and there is little benefit after LLVM anyhow) but maybe for AS it's possible.

__builtin_trap() will produce two sequenced unreachables in LLVM. So TrapsNeverHappen will remove one of them? Or all? Because we also uses assert method quite often which produce explicit unreachable which shouldn't remove

[kripken]

Yes, TNH can remove any unreachable. So it is not a good fit for LLVM, in general, yeah.

When you use unreachable for assertions in AS, would it be ok to remove those assertions in fully-optimized builds? That is the idea in TNH mode. (If you do want those assertions in production, then TNH does not make sense.)

[MaxGraey]

When you use unreachable for assertions in AS, would it be ok to remove those assertions in fully-optimized builds?

Nope. We have --noAssert flag, but usually it doesn't use in production by default.

Basically it will be great to have mode which remove all dead loads with implicit traps (and perhaps other implicit traps) but still keep explicit unreachables. Unfortunately TNH removes unreachable.

[kripken]

Maybe we can add a flag for that. But it is not a good mode, as mentioned in the TNH PR, as we do things like turn an instruction into a combination of other instructions including an unreachable. If we special-case unreachable then those opts would be making things worse rather than better, and even though the behavior is identical.

If you use something other than unreachable for assertions you want in production, that could work. One option is for Binaryen IR to add a new special instruction just for that. Another is for code to call an import to error, or throw a wasm exception (if using wasm exceptions).

[MaxGraey]

Another is for code to call an import to error, or throw a wasm exception (if using wasm exceptions).

Hmm. It could be a solution, but it will be a breaking change and you have to think about the consequences

[kripken]

Yes, all the options here have downsides, and are breaking changes. My hope is that if a compiler wants to adopt TNH mode then it could find the best of the options for it.

Thinking more on the import path, I am considering something like this:

(import "binaryen-intrinsics" "trap" (func $trap))

When optimizing, binaryen would treat that as an unreachable that it cannot remove. We could then have a --strip-intrinsics pass that lowers the import into an actual unreachable. After that point, binaryen would not be able to optimize the code as well, so this would be done at the very end before shipping. Thoughts?

[MaxGraey]

I created rfc about this. Probably we could resolve this on language/runtime design.

[MaxGraey]

Thinking more on the import path, I am considering something like this:

(import "binaryen-intrinsics" "trap" (func $trap))

I think that might work. Then AssemblyScript would not be the only one who could benefit from this mode

[tlively]

Instead of intrinsic imports, another option would be to use the code annotation framework we are designing to have a custom section to mark unreachables as intentional traps. Binaryen could represent those unreachables specially in the IR and either emit the annotation custom section or not in its output.