Stream: git-wasmtime

Topic: wasmtime / issue #5653 InsBuilder::trap_if(...) replaced ...

Wasmtime GitHub notifications bot (Jan 30 2023 at 17:16):

ptitSeb opened issue #5653:

Thanks for filing a feature request! Please fill out the TODOs below.

Feature

With 0.90 of Cranelift, InsBuilder::trap_if(...) was removed, and replaced with Insbuild::trapnz(...) and InsBuilder::trapz(...), but the trap_if(...) allowed for more case than just Equal and Different (like for example UnsignedGreaterThanOrEqual).

Benefit

Easy OutOfBound trap for example

Implementation

Add more trapXX(...) like trapnz(..)/trapz(..)

Alternatives

Manual jmp over / unconditional trap, but that's a bit cumbersome.

Wasmtime GitHub notifications bot (Jan 30 2023 at 18:02):

cfallin commented on issue #5653:

You should be able to do a trapnz on an icmp result with any comparison op, I think?

Wasmtime GitHub notifications bot (Jan 30 2023 at 18:29):

ptitSeb commented on issue #5653:

so, an icmp to get a value, than a test on that value to finaly do a trapz() or trapnz()? That's a solution, but still lot more complex then previous trap_if

Wasmtime GitHub notifications bot (Jan 30 2023 at 18:40):

bjorn3 commented on issue #5653:

You can do icmp with the IntCC for the condition in which you want to trap and then directly pass the result of icmp to trapnz. For example

trap_if uge v0, v1

would turn into

v2 = icmp uge v0, v1
trapnz v2

Wasmtime GitHub notifications bot (Jan 30 2023 at 18:40):

jameysharp commented on issue #5653:

I believe the intent was that for UnsignedGreaterThanOrEqual you'd use icmp like this:

v2 = icmp uge v0, v1
trapnz v2, user0

There are some special cases which might help, like the uadd_overflow_trap instruction, or using the carry output of iadd_cout as the input to trapnz.

Do those options cover your needs?

Wasmtime GitHub notifications bot (Jan 30 2023 at 19:03):

ptitSeb commented on issue #5653:

Ok, thanks. the icmp / trapnz is good enough. I'll check the uadd_overflow_trap too also.

Wasmtime GitHub notifications bot (Jan 30 2023 at 19:10):

ptitSeb commented on issue #5653:

I guess I can close the ticket as my understanding of trapz / trapnz was eronuous: it doesn't take a CFlag but a Value (and so it's not a direct replacement to trap_if)

Wasmtime GitHub notifications bot (Jan 30 2023 at 20:06):

jameysharp commented on issue #5653:

Right, trapif was removed as part of our effort to remove integer/floating-point CPU flags from CLIF. So yeah, I'll go ahead and close this. Thanks for asking the question though—it's always good to hear from folks who are using Cranelift!

Wasmtime GitHub notifications bot (Jan 30 2023 at 20:06):

jameysharp closed issue #5653:

Thanks for filing a feature request! Please fill out the TODOs below.

Feature

With 0.90 of Cranelift, InsBuilder::trap_if(...) was removed, and replaced with Insbuild::trapnz(...) and InsBuilder::trapz(...), but the trap_if(...) allowed for more case than just Equal and Different (like for example UnsignedGreaterThanOrEqual).

Benefit

Easy OutOfBound trap for example

Implementation

Add more trapXX(...) like trapnz(..)/trapz(..)

Alternatives

Manual jmp over / unconditional trap, but that's a bit cumbersome.

Last updated: Dec 23 2024 at 12:05 UTC