[Bug tree-optimization/68640] foo restrict propagated to foo._omp_fn.0

["vries at gcc dot gnu.org" <gcc-bugzilla at gcc dot gnu dot org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68640

--- Comment #2 from vries at gcc dot gnu.org ---
(In reply to Richard Biener from comment #1)
> Is it used because it ends up in the static chain of the omp_fn and uses the
> same (original) qualifiers?  And the static chain itself is passed "by
> reference"
> and thus gets its fields looked at and restrict extracted.

AFAIU, the static chain is only used for nested functions, and the omp_fn's are
not (though similar). So I guess you mean the single function argument pointing
to a struct that contains the base pointers used in omp_fns.

Yes, that argument points to a struct where the fields use the same qualifiers
as in the original function (although in some cases, there will be a pointer
indirection introduced inbetween, which will cause the restrict to have no
effect). And that argument is passed as a restrict reference and thus gets its
field looked at and restrict extracted (btw the same would happen if the
argument was a restrict pointer).

The restrict on the fields is set in install_var_field (by using the exact
type, rather than clearing some qualifiers).

> int
> foo (int *__restrict__ ap)
> {
>   int *bp = ap;
> #pragma omp parallel for
>   for (unsigned int idx = 0; idx < N; idx++)
>     ap[idx] = bp[idx];
> }
> 
> I believe this testcase is invalid as you access *ap through bp which is
> not based on ap.
> 

Modifying ap to point to a copy of *ap will modify the value of bp (when done
before the copy from ap to bp), so to me it looks like bp is based on ap.

> But your point is that "based on" is not properly passed through the
> OMP lowering which will split out the use into a separate function
> thus violating the constraints we set for restrict in PTA (_only_
> derive it from parameters because their scope is fixed).

Basically we redeclare ap as restrict pointer ap in the scope foo._omp_fn.0.
And bp is not based on the redeclared ap, so this violates restrict.

It's similar to rewriting this legal example:
...
{
  int *restrict ap = ...
  int *bp = ap;
  {
     int *ap2 = ap;
     *ap2 = 1;
     *bp = 2;
  }
}
...
into this illegal example:
...
{
  int *restrict ap = ...
  int *bp = ap;
  {
     int *restrict ap2 = ap;
     *ap2 = 1;
     *bp = 2;
  }
}
...
It becomes illegal by adding restrict to ap2. It is illegal because bp is not
based on ap2.

> This would mean you need to drop restrict qualifiers for any pointers
> you "leak" to the static chain (in the static chain field-decls type).
> 

Right, clearing the restrict qualifier on pointers in install_var_field would
fix this.

> Yes, if we fix visit_loadstore for
> 
>           /* ???  We need to make sure 'ptr' doesn't include any of
>              the restrict tags we added bases for in its points-to set.  */
>           return false;
> 
> you may be able to create a miscompile here (if you manage to create a
> valid testcase in the first place)

I still think the testcase is valid.