On 6 December 2016 at 14:50, Richard Biener <rguent...@suse.de> wrote:
> On Tue, 6 Dec 2016, Jakub Jelinek wrote:
>
>> On Tue, Dec 06, 2016 at 09:36:55AM +0100, Richard Biener wrote:
>> > > As shown on the testcase, with K&R definitions and fn prototypes with
>> > > promoted types, we can end up computing caller's value ranges in wider
>> > > type than the parameter actually has in the function.
>> > > The problem with that is that wide_int_storage::from can actually wrap
>> > > around, so either as in the testcase we end up with invalid range
>> > > (minimum
>> > > larger than maximum), or just with a range that doesn't cover all the
>> > > values
>> > > the parameter can have.
>> > > The patch punts if the range bounds cast to type aren't equal to the
>> > > original values. Similarly (just theoretical), for pointers it only
>> > > optimizes if the caller's precision as at most as wide as the pointer,
>> > > if it would be wider, even ~[0, 0] range could actually be a NULL pointer
>> > > (some multiple of ~(uintptr_t)0 + (uintmax_t) 1).
>> > >
>> > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>> >
>> > Ok, but I wonder whether this also addresses PR78365 which has a
>> > patch pending (to be reviewed by IPA maintainers) that makes propagation
>> > across such calls more sensible by recording type information in
>> > the jump functions.
>>
>> It is effectively a dup. So, my patch fixes the PR78365 testcase and I bet
>> (though haven't tried, but it is extremely likely) that the other patch
>> fixes PR78681 testcase.
>> So, do you want me to add the pr78365.c testcase to my patch, or prefer
>> the other patch? OT, in the other patch I've noticed incorrect formatting:
>> + parm ?
>> + TREE_TYPE (parm) :
>> NULL_TREE);
>
> I prefer the other patch (well, the other approach, didn't look into
> the patch in detail).
We would also need param types to be saved in jump function for
ipa-bits-propagation,
to fix cases like PR78599.
Thanks,
Prathamesh
>
> Richard.
>
>> > > 2016-12-05 Jakub Jelinek <ja...@redhat.com>
>> > >
>> > > PR tree-optimization/78681
>> > > * ipa-prop.c (ipcp_update_vr): Punt if vr[i].min precision is bigger
>> > > then type's precision and vr[i].min or vr[i].max in type would wrap.
>> > >
>> > > * gcc.c-torture/compile/pr78681.c: New test.
>> > >
>> > > --- gcc/ipa-prop.c.jj 2016-11-25 18:11:05.000000000 +0100
>> > > +++ gcc/ipa-prop.c 2016-12-05 18:48:48.853882864 +0100
>> > > @@ -5709,8 +5709,23 @@ ipcp_update_vr (struct cgraph_node *node
>> > > {
>> > > tree type = TREE_TYPE (ddef);
>> > > unsigned prec = TYPE_PRECISION (type);
>> > > + unsigned mprec = wi::get_precision (vr[i].min);
>> > > + gcc_assert (mprec == wi::get_precision (vr[i].max));
>> > > if (INTEGRAL_TYPE_P (TREE_TYPE (ddef)))
>> > > {
>> > > + if (prec < mprec)
>> > > + {
>> > > + /* If there is a disagreement between callers and callee
>> > > + on the argument type, e.g. when using K&R function
>> > > + definitions, punt if vr[i].min or vr[i].max are outside
>> > > + of type's precision. */
>> > > + wide_int m = wi::ext (vr[i].min, prec, TYPE_SIGN (type));
>> > > + if (m != vr[i].min)
>> > > + continue;
>> > > + m = wi::ext (vr[i].max, prec, TYPE_SIGN (type));
>> > > + if (m != vr[i].max)
>> > > + continue;
>> > > + }
>> > > if (dump_file)
>> > > {
>> > > fprintf (dump_file, "Setting value range of param %u ", i);
>> > > @@ -5729,6 +5744,7 @@ ipcp_update_vr (struct cgraph_node *node
>> > > }
>> > > else if (POINTER_TYPE_P (TREE_TYPE (ddef))
>> > > && vr[i].type == VR_ANTI_RANGE
>> > > + && mprec <= prec
>> > > && wi::eq_p (vr[i].min, 0)
>> > > && wi::eq_p (vr[i].max, 0))
>> > > {
>> > > --- gcc/testsuite/gcc.c-torture/compile/pr78681.c.jj 2016-12-05
>> > > 19:51:15.353646309 +0100
>> > > +++ gcc/testsuite/gcc.c-torture/compile/pr78681.c 2016-12-05
>> > > 19:50:57.000000000 +0100
>> > > @@ -0,0 +1,27 @@
>> > > +/* PR tree-optimization/78681 */
>> > > +
>> > > +struct S { char b; };
>> > > +char d, e, f, l, m;
>> > > +struct S n;
>> > > +int bar (char, char);
>> > > +static void foo (struct S *, int, int, int, int);
>> > > +
>> > > +static void
>> > > +foo (x, g, h, i, j)
>> > > + struct S *x;
>> > > + char g, h, i, j;
>> > > +{
>> > > + char k;
>> > > + for (k = 0; k <= j; k++)
>> > > + if (bar (g, k))
>> > > + for (; i; k++)
>> > > + if (d)
>> > > + x->b = g;
>> > > +}
>> > > +
>> > > +void
>> > > +baz (int q)
>> > > +{
>> > > + foo (&n, m, l, f, 1);
>> > > + foo (&n, m, e, f, e - 1);
>> > > +}
>>
>> Jakub
>>
>>
>
> --
> Richard Biener <rguent...@suse.de>
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
> 21284 (AG Nuernberg)
