Re: [Valgrind-developers] [valgrind] Bug 404406 - s390x: implement z14 miscellaneous instructions

[Mark W. <ma...@kl...>]

Hi,

On Wed, 2019-06-12 at 18:24 +0000, Andreas Arnez wrote:
> https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=50b20aa244c1fddd9b9e872526d4daad49ddd7a5
> 
> commit 50b20aa244c1fddd9b9e872526d4daad49ddd7a5
> Author: Ilya Leoshkevich <ii...@li...>
> Date:   Thu May 16 12:34:37 2019 +0200
> 
>     Bug 404406 - s390x: implement z14 miscellaneous instructions
>     
>     (from bug 404406 comment 0):
>     Valgrind on s390x currently lacks support for the miscellaneous
>     instruction extensions facility 2.
>     
>     Signed-off-by: Ilya Leoshkevich <ii...@li...>

Sorry, this is a bit old, but I am finally testing this and on a z13
setup (no z14) this part causes trouble:
 
> diff --git a/VEX/priv/host_s390_isel.c b/VEX/priv/host_s390_isel.c
> index 9141b7b..30e5c76 100644
> --- a/VEX/priv/host_s390_isel.c
> +++ b/VEX/priv/host_s390_isel.c
> @@ -1018,6 +1018,8 @@ s390_isel_int128_expr_wrk(HReg *dst_hi, HReg *dst_lo, ISelEnv *env,
>           goto do_multiply64;
>  
>        case Iop_MullS64:
> +         if (!(env->hwcaps & VEX_HWCAPS_S390X_MI2))
> +            goto irreducible;
>           is_signed_multiply = True;
>           goto do_multiply64;
>  
> @@ -1125,7 +1127,10 @@ s390_isel_int128_expr_wrk(HReg *dst_hi, HReg *dst_lo, ISelEnv *env,
>        }
>     }
>  
> -   vpanic("s390_isel_int128_expr");
> +   /* We get here if no pattern matched. */
> + irreducible:
> +   ppIRExpr(expr);
> +   vpanic("s390_isel_int128_expr: cannot reduce tree");
>  }

This seems to be a new restriction for Iop_MullS64, which seems to
reduce fine before. With this the following testcases start failing on
z13:

none/tests/s390x/vector                  (stdout)
none/tests/s390x/vector                  (stderr)
none/tests/s390x/vector_float            (stdout)
none/tests/s390x/vector_float            (stderr)
none/tests/s390x/vector_integer          (stdout)
none/tests/s390x/vector_integer          (stderr)
none/tests/s390x/vector_string           (stdout)
none/tests/s390x/vector_string           (stderr)

Removing that hunk makes them pass again.

Is this really necessary for z14? If so, that check is probably in the
wrong place or testing the wrong facility.

Thanks,

Mark