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.
