This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PR35503 - warn for restrict pointer


On 1 September 2016 at 12:25, Richard Biener <rguenther@suse.de> wrote:
> 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.
Hi,
The attached version passes bootstrap+test on ppc64le-linux-gnu.
Given that it only looks if parameters are restrict qualified and not
how they're used inside the callee,
this can have false positives as in above test-cases.
Should the warning be put in Wextra rather than Wall (I have left it
in Wall in the patch)  or only enabled with -Wrestrict ?

Thanks,
Prathamesh
>
> Richard.

Attachment: pr35503-7.diff
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]