On Tue, 16 Apr 2019, Jakub Jelinek wrote:
> On Tue, Apr 16, 2019 at 06:21:25PM +0200, Eric Botcazou wrote:
> > > The runtime check assures that at runtime, the upper 32 bits of pseudo 104
> > > must be always 0 (in this case, in some other case could be sign bit
> > > copies).
> >
> > OK, as Richard pointed out, that's not sufficient if we allow...
> >
> > > The question is if it would be valid say for forward propagation to first
> > > propagate (or combine) the pseudo 97 into the (subreg/s/v:SI (reg:DI 104)
> > > 0), then hoisting it before the jump_insn 16, have the subreg optimized
> > > away and miscompile later on.
> >
> > ...this to happen. So we could clear SUBREG_PROMOTED_VAR_P as soon as the
> > SUBREG is rewritten, but this looks quite fragile. The safest route is
> > probably not to use SUBREG_PROMOTED_VAR_P in this conditional context.
> >
> > > That means either that the hoisting pass is buggy, or that
> > > SUBREG_PROMOTED_*
> > > is only safe at the function boundary (function arguments and return
> > > value)
> > > and not elsewhere.
> >
> > I think that Richard's characterization is correct:
> >
> > "Note that likely SUBREG_PROMOTED_VAR_P wasn't designed to communicate
> > zero-extend info (can't you use a REG_EQUIV note somehow?) but it has
> > to be information that is valid everywhere in the function unless
> > data dependences force its motion (thus a conditional doesn't do)."
> >
> > i.e. this also works for a local variable that is always accessed with the
> > SUBREG_PROMOTED_VAR_P semantics.
>
> Ok, here is a patch that just removes all of that SUBREG_PROMOTED_SET then,
> as even for the opN_small_p we can't actually guarantee that for the whole
> function, only for where the pseudo with the SSA_NAME for which we get the
> range appears. On the bright side, the generated code at least for the
> particular testcase has somewhat different RA decisions, but isn't
> significantly worse.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Richard.
> 2019-04-16 Jakub Jelinek <ja...@redhat.com>
>
> PR middle-end/90095
> * internal-fn.c (expand_mul_overflow): Don't set SUBREG_PROMOTED_VAR_P
> on lowpart SUBREGs.
>
> * gcc.dg/pr90095-1.c: New test.
> * gcc.dg/pr90095-2.c: New test.
>
> --- gcc/internal-fn.c.jj 2019-04-15 19:45:22.384444646 +0200
> +++ gcc/internal-fn.c 2019-04-16 15:18:56.614708804 +0200
> @@ -1753,22 +1753,9 @@ expand_mul_overflow (location_t loc, tre
> /* If both op0 and op1 are sign (!uns) or zero (uns) extended from
> hmode to mode, the multiplication will never overflow. We can
> do just one hmode x hmode => mode widening multiplication. */
> - rtx lopart0s = lopart0, lopart1s = lopart1;
> - if (GET_CODE (lopart0) == SUBREG)
> - {
> - lopart0s = shallow_copy_rtx (lopart0);
> - SUBREG_PROMOTED_VAR_P (lopart0s) = 1;
> - SUBREG_PROMOTED_SET (lopart0s, uns ? SRP_UNSIGNED : SRP_SIGNED);
> - }
> - if (GET_CODE (lopart1) == SUBREG)
> - {
> - lopart1s = shallow_copy_rtx (lopart1);
> - SUBREG_PROMOTED_VAR_P (lopart1s) = 1;
> - SUBREG_PROMOTED_SET (lopart1s, uns ? SRP_UNSIGNED : SRP_SIGNED);
> - }
> tree halfstype = build_nonstandard_integer_type (hprec, uns);
> - ops.op0 = make_tree (halfstype, lopart0s);
> - ops.op1 = make_tree (halfstype, lopart1s);
> + ops.op0 = make_tree (halfstype, lopart0);
> + ops.op1 = make_tree (halfstype, lopart1);
> ops.code = WIDEN_MULT_EXPR;
> ops.type = type;
> rtx thisres
> --- gcc/testsuite/gcc.dg/pr90095-1.c.jj 2019-04-16 13:45:22.614772955
> +0200
> +++ gcc/testsuite/gcc.dg/pr90095-1.c 2019-04-16 13:45:22.614772955 +0200
> @@ -0,0 +1,18 @@
> +/* PR middle-end/90095 */
> +/* { dg-do run } */
> +/* { dg-options "-Os -fno-tree-bit-ccp" } */
> +
> +unsigned long long a;
> +unsigned int b;
> +
> +int
> +main ()
> +{
> + unsigned int c = 255, d = c |= b;
> + if (__CHAR_BIT__ != 8 || __SIZEOF_INT__ != 4 || __SIZEOF_LONG_LONG__ != 8)
> + return 0;
> + d = __builtin_mul_overflow (-(unsigned long long) d, (unsigned char) - c,
> &a);
> + if (d != 0)
> + __builtin_abort ();
> + return 0;
> +}
> --- gcc/testsuite/gcc.dg/pr90095-2.c.jj 2019-04-16 15:20:14.728414325
> +0200
> +++ gcc/testsuite/gcc.dg/pr90095-2.c 2019-04-16 15:20:29.597167928 +0200
> @@ -0,0 +1,5 @@
> +/* PR middle-end/90095 */
> +/* { dg-do run } */
> +/* { dg-options "-Os -fno-tree-bit-ccp -fno-split-wide-types" } */
> +
> +#include "pr90095-1.c"
>
>
> Jakub
>
--
Richard Biener <rguent...@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)
