[2/2] tcg: Add missing tcg_can_emit_vec_op check in tcg_gen_gvec_2s

Message ID 20180217164037.15727-3-richard.henderson@linaro.org
State New
Headers show
Series
  • tcg: tcg_can_emit_vec_op cleanup+fix
Related show

Commit Message

Richard Henderson Feb. 17, 2018, 4:40 p.m.
This lead to an assertion failure for 64-bit vector multiply,
which is not available in the AVX instruction set.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 tcg/tcg-op-gvec.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

-- 
2.14.3

Comments

Peter Maydell Feb. 22, 2018, 2:58 p.m. | #1
On 17 February 2018 at 16:40, Richard Henderson
<richard.henderson@linaro.org> wrote:
> This lead to an assertion failure for 64-bit vector multiply,

> which is not available in the AVX instruction set.

>

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>  tcg/tcg-op-gvec.c | 9 ++++++---

>  1 file changed, 6 insertions(+), 3 deletions(-)

>

> diff --git a/tcg/tcg-op-gvec.c b/tcg/tcg-op-gvec.c

> index 29f9cf34b4..432e577c35 100644

> --- a/tcg/tcg-op-gvec.c

> +++ b/tcg/tcg-op-gvec.c

> @@ -979,12 +979,15 @@ void tcg_gen_gvec_2s(uint32_t dofs, uint32_t aofs, uint32_t oprsz,

>

>      type = 0;

>      if (g->fniv) {

> -        if (TCG_TARGET_HAS_v256 && check_size_impl(oprsz, 32)) {

> +        if (TCG_TARGET_HAS_v256 && check_size_impl(oprsz, 32)

> +            && tcg_can_emit_vec_op(g->opc, TCG_TYPE_V256, g->vece)) {

>              type = TCG_TYPE_V256;

> -        } else if (TCG_TARGET_HAS_v128 && check_size_impl(oprsz, 16)) {

> +        } else if (TCG_TARGET_HAS_v128 && check_size_impl(oprsz, 16)

> +                   && tcg_can_emit_vec_op(g->opc, TCG_TYPE_V128, g->vece)) {

>              type = TCG_TYPE_V128;

>          } else if (TCG_TARGET_HAS_v64 && !g->prefer_i64

> -               && check_size_impl(oprsz, 8)) {

> +                   && check_size_impl(oprsz, 8)

> +                   && tcg_can_emit_vec_op(g->opc, TCG_TYPE_V64, g->vece)) {

>              type = TCG_TYPE_V64;

>          }

>      }


Reviewed-by: Peter Maydell <peter.maydell@linaro.org>


Incidentally, I notice that the condition checks
     (TCG_TARGET_HAS_v256 && check_size_impl(oprsz, 32)
      && tcg_can_emit_vec_op(g->opc, TCG_TYPE_V256, g->vece))

     (TCG_TARGET_HAS_v128 && check_size_impl(oprsz, 16)
      && tcg_can_emit_vec_op(g->opc, TCG_TYPE_V128, g->vece))

     (TCG_TARGET_HAS_v64 && check_size_impl(oprsz, 8)
      && tcg_can_emit_vec_op(g->opc, TCG_TYPE_V64, g->vece))

seem to be quite commonly used -- perhaps factoring these out
into suitably named functions would make the code more readable?

thanks
-- PMM
Richard Henderson Feb. 22, 2018, 3:32 p.m. | #2
On 02/22/2018 06:58 AM, Peter Maydell wrote:
> Incidentally, I notice that the condition checks

>      (TCG_TARGET_HAS_v256 && check_size_impl(oprsz, 32)

>       && tcg_can_emit_vec_op(g->opc, TCG_TYPE_V256, g->vece))

> 

>      (TCG_TARGET_HAS_v128 && check_size_impl(oprsz, 16)

>       && tcg_can_emit_vec_op(g->opc, TCG_TYPE_V128, g->vece))

> 

>      (TCG_TARGET_HAS_v64 && check_size_impl(oprsz, 8)

>       && tcg_can_emit_vec_op(g->opc, TCG_TYPE_V64, g->vece))

> 

> seem to be quite commonly used -- perhaps factoring these out

> into suitably named functions would make the code more readable?


That's a good idea.  I'll see what I can come up with there.


r~

Patch

diff --git a/tcg/tcg-op-gvec.c b/tcg/tcg-op-gvec.c
index 29f9cf34b4..432e577c35 100644
--- a/tcg/tcg-op-gvec.c
+++ b/tcg/tcg-op-gvec.c
@@ -979,12 +979,15 @@  void tcg_gen_gvec_2s(uint32_t dofs, uint32_t aofs, uint32_t oprsz,
 
     type = 0;
     if (g->fniv) {
-        if (TCG_TARGET_HAS_v256 && check_size_impl(oprsz, 32)) {
+        if (TCG_TARGET_HAS_v256 && check_size_impl(oprsz, 32)
+            && tcg_can_emit_vec_op(g->opc, TCG_TYPE_V256, g->vece)) {
             type = TCG_TYPE_V256;
-        } else if (TCG_TARGET_HAS_v128 && check_size_impl(oprsz, 16)) {
+        } else if (TCG_TARGET_HAS_v128 && check_size_impl(oprsz, 16)
+                   && tcg_can_emit_vec_op(g->opc, TCG_TYPE_V128, g->vece)) {
             type = TCG_TYPE_V128;
         } else if (TCG_TARGET_HAS_v64 && !g->prefer_i64
-               && check_size_impl(oprsz, 8)) {
+                   && check_size_impl(oprsz, 8)
+                   && tcg_can_emit_vec_op(g->opc, TCG_TYPE_V64, g->vece)) {
             type = TCG_TYPE_V64;
         }
     }