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?
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
