Re: PR35503 - warn for restrict pointer

[Richard Biener <rguenther at suse dot de>]

On Tue, 30 Aug 2016, Tom de Vries wrote:

> On 30/08/16 17:34, Prathamesh Kulkarni wrote:
> > On 30 August 2016 at 20:24, Tom de Vries <Tom_deVries@mentor.com> wrote:
> > > On 26/08/16 13:39, Prathamesh Kulkarni wrote:
> > > > 
> > > > Hi,
> > > > The following patch adds option -Wrestrict that warns when an argument
> > > > is passed to a restrict qualified parameter and it aliases with
> > > > another argument.
> > > > 
> > > > eg:
> > > > int foo (const char *__restrict buf, const char *__restrict fmt, ...);
> > > > 
> > > > void f(void)
> > > > {
> > > >   char buf[100] = "hello";
> > > >   foo (buf, "%s-%s", buf, "world");
> > > > }
> > > 
> > > 
> > > Does -Wrestrict generate a warning for this example?
> > > 
> > > ...
> > > void h(int n, int * restrict p, int * restrict q, int * restrict r)
> > > {
> > >   int i;
> > >   for (i = 0; i < n; i++)
> > >     p[i] = q[i] + r[i];
> > > }
> > > 
> > > h (100, a, b, b)
> > > ...
> > > 
> > > [ Note that this is valid C, and does not violate the restrict definition.
> > > ]
> > > 
> > > If -Wrestrict indeed generates a warning, then we should be explicit in
> > > the
> > > documentation that while the warning triggers on this type of example, the
> > > code is correct.
> > I am afraid it would warn for the above case. The patch just checks if
> > the parameter is qualified
> > with restrict, and if the corresponding argument has aliases in the
> > call (by calling operand_equal_p).
> 
> > Is such code common in practice ?
> 
> [ The example is from the definition of restrict in the c99 standard. ]
> 
> According to the definition of restrict, only the restrict on p is required to
> know that p doesn't alias with q and that p doesn't alias with r.
> 
> AFAIK the current implementation of gcc already generates optimal code if
> restrict is only on p. But earlier versions (and possibly other compilers as
> well?) need the restrict on q and r as well.
> 
> So I expect this code to occur.
> 
> > If it is, I wonder if we should keep
> > the warning in -Wall ?
> > 
> 
> Hmm, I wonder if we can use the const keyword to silence the warning.
> 
> So if we generate a warning for:
> ...
> void h(int n, int * restrict p, int * restrict q, int * restrict r)
> {
>   int i;
>   for (i = 0; i < n; i++)
>     p[i] = q[i] + r[i];
> }
> h (100, a, b, b)
> ...
> 
> but not for:
> ...
> void h(int n, int * restrict p, const int * restrict q, const int * restrict
> r)
> {
>   int i;
>   for (i = 0; i < n; i++)
>     p[i] = q[i] + r[i];
> }
> h (100, a, b, b)
> ...
> 
> Then there's an easy way to rewrite the example such that the warning doesn't
> trigger, without having to remove the restrict.

Note that I've seen restrict being used even for

void h(int n, int * restrict p, int * restrict q)
{
  int i;
  for (i = 0; i < n; i++)
    p[2*i] = p[2*i] + q[2*i + 1];
}

thus where the actual accesses do not alias (the pointers are used
to access every other element).  I think the definition of "object"
(based on accessed lvalues) makes this example valid.  So we
shouldn't warn about

h (n, p, p)

but we could warn about

h (n, p, p + 1)

and yes, only one of the pointers need to be restrict qualified.

Note that as with all other alias warnings if you want to avoid
false positives and rely on conservative analysis then you can
as well simply avoid taking advantate of the bug in the code
(as we do for TBAA and trivial must-alias cases).  If you allow
false positives then you ultimatively end up with a mess like
our existing -Wstrict-aliasing warnings (which are IMHO quite
useless).

Note that nowhere in GCC we implement anything closely matching
the formal definition of restrict as writte in the C standard --
only in fronted code could we properly derive the 'based-on'
property and note lvalues affected by restrict.  Currently we
are restricted to looking at restrict qualified parameters
because of this.

Richard.