Wasmtime GitHub notifications bot (Sep 23 2021 at 13:55): Wasmtime GitHub notifications bot (Sep 23 2021 at 13:55):afonso360 opened PR #3384 from aarch64-bmask to main:
Hey, this PR Implements
bmaskfor all types in the AArch64 backend, as well as fixes forireduce,breduceandbextendfor some types.We have to make some changes in the trampoline to make bool args with size larger than 1 work.
Wasmtime GitHub notifications bot (Sep 23 2021 at 15:47):akirilov-arm submitted PR review.
Wasmtime GitHub notifications bot (Sep 23 2021 at 15:47):akirilov-arm created PR review comment:
I am afraid that this is not true for vector types; e.g. consider an operation from
B8X8toB16X8.Unless both the input and the output have the same lane sizes (the number of lanes is always the same, as specified by the operation descriptions), you will have to write much more complicated logic, and will probably be unable to merge the scalar and vector code paths, as you do now.
Wasmtime GitHub notifications bot (Sep 23 2021 at 15:47):akirilov-arm submitted PR review.
Wasmtime GitHub notifications bot (Sep 23 2021 at 16:04):afonso360 submitted PR review.
Wasmtime GitHub notifications bot (Sep 23 2021 at 16:04):afonso360 created PR review comment:
Right, I didn't really prepare this for those types (non 128 bit vectors). I haven't really seen them used anywhere so far.
Would it be acceptable to assert on 128bit vectors for now?
Wasmtime GitHub notifications bot (Sep 23 2021 at 17:32):akirilov-arm submitted PR review.
Wasmtime GitHub notifications bot (Sep 23 2021 at 17:32):akirilov-arm created PR review comment:
No, just keep the block (lines 1864 - 1869) that you have removed. Of course, you will need to change the condition - I suppose to
from_ty.is_vector() && (from_bits != 128 || from_bits != to_bits).IMHO assertions should be used for verifying invariants guaranteed by the IR descriptions, e.g. checking that the number of input lanes is equal to the number of output lanes. That is the approach I have used in my recent changes.
Wasmtime GitHub notifications bot (Sep 23 2021 at 17:40):akirilov-arm edited PR review comment.
Wasmtime GitHub notifications bot (Sep 23 2021 at 17:43):akirilov-arm edited PR review comment.
Wasmtime GitHub notifications bot (Sep 23 2021 at 17:43):akirilov-arm edited PR review comment.
Wasmtime GitHub notifications bot (Sep 24 2021 at 17:31):afonso360 updated PR #3384 from aarch64-bmask to main.
Wasmtime GitHub notifications bot (Sep 24 2021 at 21:36):akirilov-arm submitted PR review.
Wasmtime GitHub notifications bot (Sep 24 2021 at 21:36):akirilov-arm created PR review comment:
Given that
to_bits > from_bits, we know thatfrom_bits <= 64, so there is no need to calculate a minimum here.
Wasmtime GitHub notifications bot (Sep 24 2021 at 21:36):akirilov-arm created PR review comment:
We could also be lowering
Ireduce- see below.
Wasmtime GitHub notifications bot (Sep 24 2021 at 21:36):akirilov-arm submitted PR review.
Wasmtime GitHub notifications bot (Sep 24 2021 at 21:36):akirilov-arm created PR review comment:
Remove this assertion -
BreducefromB128toB128andIreducefromI128toI128are also valid operations.
Wasmtime GitHub notifications bot (Sep 24 2021 at 21:36):akirilov-arm created PR review comment:
I feel like this should be calculated in parallel with the bottom half, at the expense of a bit of code duplication, especially in the case of
from_bits == to_bits.
Wasmtime GitHub notifications bot (Sep 24 2021 at 21:49):akirilov-arm submitted PR review.
Wasmtime GitHub notifications bot (Sep 24 2021 at 21:49):akirilov-arm created PR review comment:
Even after your latest changes, this comment should be changed to
Smaller scalar integers/booleans are....
Wasmtime GitHub notifications bot (Sep 24 2021 at 21:52):akirilov-arm submitted PR review.
Wasmtime GitHub notifications bot (Sep 24 2021 at 21:52):akirilov-arm created PR review comment:
Now that I think about it,
IreducefromI128toI128needs completely different treatment - the upper destination register is not a copy of the lower source register.
Wasmtime GitHub notifications bot (Sep 24 2021 at 21:57):akirilov-arm edited PR review comment.
Wasmtime GitHub notifications bot (Oct 07 2021 at 15:50):afonso360 submitted PR review.
Wasmtime GitHub notifications bot (Oct 07 2021 at 15:50):afonso360 created PR review comment:
The docs state that
ireducerequires that the output type is smaller than the input type, so at most we only ever have ai64output (if we get ai128input). I've checked and the verifier enforces this.breducehas the same language in the docs.
Wasmtime GitHub notifications bot (Oct 07 2021 at 17:04):akirilov-arm created PR review comment:
Well, then there is a contradiction because the documentation right now states:
The result type must have the same number of vector lanes as the input, and each lane must not have more bits that the input lanes.
In other words, there are two options - either the same number of bits per lane, or fewer. If indeed the verifier enforces the second option, then do you mind making the description less ambiguous? Same for
Breduce.
Wasmtime GitHub notifications bot (Oct 07 2021 at 17:04):akirilov-arm submitted PR review.
Wasmtime GitHub notifications bot (Jan 26 2022 at 16:03):afonso360 closed without merge PR #3384.
Last updated: Dec 06 2025 at 06:05 UTC