On Thu, 30 Aug 2018, Jakub Jelinek wrote:
> On Thu, Aug 30, 2018 at 09:57:48AM +0200, Richard Biener wrote:
> > > > Ugh. I wonder if we should add a
> > > >
> > > > rtx gen_int_mode (poly_uint64 c, machine_mode mode)
> > > >
> > > > and assert that the topmost bit is not set if GET_MODE_PRECISION (mode)
> > > > > 64?
> > > >
> > > > But I guess passing unsigned HOST_WIDE_INT will make this ambiguous.
> > > > So maybe a unsigned HOST_WIDE_INT overload instead.
> > >
> > > I can try that, but I think there might be false positives too,
>
> Neither works, if there is
> rtx gen_int_mode (poly_int64 c, machine_mode mode);
> rtx gen_int_mode (poly_uint64 c, machine_mode mode);
> it is indeed ambiguous, if it is:
> rtx gen_int_mode (poly_int64 c, machine_mode mode);
> rtx gen_int_mode (unsigned HOST_WIDE_INT c, machine_mode mode);
> then the latter is called when the argument is signed HOST_WIDE_INT and
> the former is called when the argument is poly_uint64.
>
> > > If you think for maintainance it is better to just do that
> > > unconditionally,
> > > I can certainly retest.
> >
> > Yes, I think so. It also makes it explicit wheter we want an unsigned
> > or signed value.
>
> So like this if it passes bootstrap/regtest?
Yes.
Thanks,
Richard.
> I've so far verified it does the right thing in this
> unsigned __int128 x; ... x * 0x7fffffffffffffff;
> case (use (const_wide_int 0x08000000000000000)), and with
> unsigned __int128 x; ... x * 0x7fffffff;
> case (use (const_int 0x80000000)), and with
> unsigned long x; ... x * 0x7fffffffffffffff;
> case (use (const_int 0x8000000000000000)).
>
> 2018-08-30 Jakub Jelinek <ja...@redhat.com>
>
> PR middle-end/87138
> * expmed.c (expand_mult_const): Use immed_wide_int_const instead of
> gen_int_mode. Formatting fixes.
>
> * gcc.target/i386/avx512bw-pr87138.c: New test.
>
> --- gcc/expmed.c.jj 2018-08-29 23:36:11.854187906 +0200
> +++ gcc/expmed.c 2018-08-30 11:38:01.151850590 +0200
> @@ -3347,19 +3347,21 @@ expand_mult_const (machine_mode mode, rt
> /* Write a REG_EQUAL note on the last insn so that we can cse
> multiplication sequences. Note that if ACCUM is a SUBREG,
> we've set the inner register and must properly indicate that. */
> - tem = op0, nmode = mode;
> - accum_inner = accum;
> - if (GET_CODE (accum) == SUBREG)
> + tem = op0, nmode = mode;
> + accum_inner = accum;
> + if (GET_CODE (accum) == SUBREG)
> {
> accum_inner = SUBREG_REG (accum);
> nmode = GET_MODE (accum_inner);
> tem = gen_lowpart (nmode, op0);
> }
>
> - insn = get_last_insn ();
> - set_dst_reg_note (insn, REG_EQUAL,
> - gen_rtx_MULT (nmode, tem,
> - gen_int_mode (val_so_far, nmode)),
> + insn = get_last_insn ();
> + wide_int wval_so_far
> + = wi::uhwi (val_so_far,
> + GET_MODE_PRECISION (as_a <scalar_mode> (nmode)));
> + rtx c = immed_wide_int_const (wval_so_far, nmode);
> + set_dst_reg_note (insn, REG_EQUAL, gen_rtx_MULT (nmode, tem, c),
> accum_inner);
> }
> }
> --- gcc/testsuite/gcc.target/i386/avx512bw-pr87138.c.jj 2018-08-30
> 11:37:23.833464733 +0200
> +++ gcc/testsuite/gcc.target/i386/avx512bw-pr87138.c 2018-08-30
> 11:37:23.833464733 +0200
> @@ -0,0 +1,29 @@
> +/* PR middle-end/87138 */
> +/* { dg-do run { target int128 } } */
> +/* { dg-options "-O -fno-tree-fre -mavx512bw -mtune=k8" } */
> +/* { dg-require-effective-target avx512bw } */
> +
> +#include "avx512bw-check.h"
> +
> +typedef int U __attribute__ ((vector_size (64)));
> +typedef __int128 V __attribute__ ((vector_size (64)));
> +V g, i;
> +
> +static inline void
> +foo (unsigned h, V j, U k, V n)
> +{
> + k /= h;
> + __builtin_memmove (&h, &n, 1);
> + n[j[1]] *= 0x7FFFFFFFFFFFFFFF;
> + j[k[5]] = 0;
> + g = n;
> + i = h + j + n;
> +}
> +
> +void
> +avx512bw_test ()
> +{
> + foo (~0, (V) { }, (U) { 5 }, (V) { 3 });
> + if (g[0] != (__int128) 3 * 0x7FFFFFFFFFFFFFFF)
> + abort ();
> +}
>
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
