| Message ID | 20240321035816.99983-1-eric.huang@linux.alibaba.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] target/riscv: Fix the element agnostic function problem | expand |
On 2024/3/21 11:58, Huang Tao wrote: > In RVV and vcrypto instructions, the masked and tail elements are set to 1s > using vext_set_elems_1s function if the vma/vta bit is set. It is the element > agnostic policy. > > However, this function can't deal the big endian situation. This patch fixes > the problem by adding handling of such case. > > Signed-off-by: Huang Tao <eric.huang@linux.alibaba.com> > Suggested-by: Richard Henderson <richard.henderson@linaro.org> > --- > Changes in v2: > - Keep the api of vext_set_elems_1s > - Reduce the number of patches. > --- > target/riscv/vector_internals.c | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/target/riscv/vector_internals.c b/target/riscv/vector_internals.c > index 12f5964fbb..3e45b9b4a7 100644 > --- a/target/riscv/vector_internals.c > +++ b/target/riscv/vector_internals.c > @@ -30,6 +30,28 @@ void vext_set_elems_1s(void *base, uint32_t is_agnostic, uint32_t cnt, > if (tot - cnt == 0) { > return ; > } > + > +#if HOST_BIG_ENDIAN > + /* > + * Deal the situation when the elements are insdie > + * only one uint64 block including setting the > + * masked-off element. > + */ > + if ((tot - 1) ^ cnt < 8) { > + memset(base + H1(tot - 1), -1, tot - cnt); > + return; > + } > + /* > + * Otherwise, at least cross two uint64_t blocks. > + * Set first unaligned block. > + */ > + if (cnt % 8 != 0) { > + uint32_t j = ROUND_UP(cnt, 8); > + memset(base + H1(j - 1), -1, j - cnt); > + cnt = j; > + } > + /* Set other 64bit aligend blocks */ > +#endif Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com> Zhiwei > memset(base + cnt, -1, tot - cnt); > } >
On 3/20/24 17:58, Huang Tao wrote: > In RVV and vcrypto instructions, the masked and tail elements are set to 1s > using vext_set_elems_1s function if the vma/vta bit is set. It is the element > agnostic policy. > > However, this function can't deal the big endian situation. This patch fixes > the problem by adding handling of such case. > > Signed-off-by: Huang Tao <eric.huang@linux.alibaba.com> > Suggested-by: Richard Henderson <richard.henderson@linaro.org> > --- > Changes in v2: > - Keep the api of vext_set_elems_1s > - Reduce the number of patches. > --- > target/riscv/vector_internals.c | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/target/riscv/vector_internals.c b/target/riscv/vector_internals.c > index 12f5964fbb..3e45b9b4a7 100644 > --- a/target/riscv/vector_internals.c > +++ b/target/riscv/vector_internals.c > @@ -30,6 +30,28 @@ void vext_set_elems_1s(void *base, uint32_t is_agnostic, uint32_t cnt, > if (tot - cnt == 0) { > return ; > } > + > +#if HOST_BIG_ENDIAN > + /* > + * Deal the situation when the elements are insdie > + * only one uint64 block including setting the > + * masked-off element. > + */ > + if ((tot - 1) ^ cnt < 8) { > + memset(base + H1(tot - 1), -1, tot - cnt); > + return; > + } (1) tot will always be a multiple of 8, afaik, so there's no need for this first block. (2) Using if not #if means that the code is always compile-tested, even if it is eliminated. r~
On 2024/3/21 16:18, Richard Henderson wrote: > On 3/20/24 17:58, Huang Tao wrote: >> In RVV and vcrypto instructions, the masked and tail elements are set >> to 1s >> using vext_set_elems_1s function if the vma/vta bit is set. It is the >> element >> agnostic policy. >> >> However, this function can't deal the big endian situation. This >> patch fixes >> the problem by adding handling of such case. >> >> Signed-off-by: Huang Tao <eric.huang@linux.alibaba.com> >> Suggested-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> Changes in v2: >> - Keep the api of vext_set_elems_1s >> - Reduce the number of patches. >> --- >> target/riscv/vector_internals.c | 22 ++++++++++++++++++++++ >> 1 file changed, 22 insertions(+) >> >> diff --git a/target/riscv/vector_internals.c >> b/target/riscv/vector_internals.c >> index 12f5964fbb..3e45b9b4a7 100644 >> --- a/target/riscv/vector_internals.c >> +++ b/target/riscv/vector_internals.c >> @@ -30,6 +30,28 @@ void vext_set_elems_1s(void *base, uint32_t >> is_agnostic, uint32_t cnt, >> if (tot - cnt == 0) { >> return ; >> } >> + >> +#if HOST_BIG_ENDIAN >> + /* >> + * Deal the situation when the elements are insdie >> + * only one uint64 block including setting the >> + * masked-off element. >> + */ >> + if ((tot - 1) ^ cnt < 8) { >> + memset(base + H1(tot - 1), -1, tot - cnt); >> + return; >> + } > > (1) tot will always be a multiple of 8, afaik, so there's no need for > this first block. > (2) Using if not #if means that the code is always compile-tested, > even if it is eliminated. > > > r~ tot is not always be a multiple of 8. In the vector instructions, the helper fuinctions will use vext_set_elems_1s to set one masked-off element. In that case, tot = cnt + esz, and tot is not the end of a vector register. There is an example in GEN_VEXT_SHIFT_VV: for (i = env->vstart; i < vl; i++) { if (!vm && !vext_elem_mask(v0, i)) { /* set masked-off elements to 1s */ vext_set_elems_1s(vd, vma, i * esz, (i + 1) * esz); continue; } As for the second point, I will use if instead of #if in the next version. Thanks, Huang Tao
diff --git a/target/riscv/vector_internals.c b/target/riscv/vector_internals.c index 12f5964fbb..3e45b9b4a7 100644 --- a/target/riscv/vector_internals.c +++ b/target/riscv/vector_internals.c @@ -30,6 +30,28 @@ void vext_set_elems_1s(void *base, uint32_t is_agnostic, uint32_t cnt, if (tot - cnt == 0) { return ; } + +#if HOST_BIG_ENDIAN + /* + * Deal the situation when the elements are insdie + * only one uint64 block including setting the + * masked-off element. + */ + if ((tot - 1) ^ cnt < 8) { + memset(base + H1(tot - 1), -1, tot - cnt); + return; + } + /* + * Otherwise, at least cross two uint64_t blocks. + * Set first unaligned block. + */ + if (cnt % 8 != 0) { + uint32_t j = ROUND_UP(cnt, 8); + memset(base + H1(j - 1), -1, j - cnt); + cnt = j; + } + /* Set other 64bit aligend blocks */ +#endif memset(base + cnt, -1, tot - cnt); }
In RVV and vcrypto instructions, the masked and tail elements are set to 1s using vext_set_elems_1s function if the vma/vta bit is set. It is the element agnostic policy. However, this function can't deal the big endian situation. This patch fixes the problem by adding handling of such case. Signed-off-by: Huang Tao <eric.huang@linux.alibaba.com> Suggested-by: Richard Henderson <richard.henderson@linaro.org> --- Changes in v2: - Keep the api of vext_set_elems_1s - Reduce the number of patches. --- target/riscv/vector_internals.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)