[Bug c/105294] New: restrict pointer - disagreement with specification

[Bug c/105294] New: restrict pointer - disagreement with specification

[muecker at gwdg dot de]

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

            Bug ID: 105294
           Summary: restrict pointer - disagreement with specification
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: muecker at gwdg dot de
  Target Milestone: ---

Here is an example where GCC/Clang assume that variables
may not alias where the C spec seems to say otherwise.

This example is based on one found by Hubert Tong
after discussions with him (and Ralf Jung).

Probably the spec should be changed instead of the
compilers.

Note that when removing the first restrict, clang but
not GCC still does the optimization, which hints at
a missed optimization opportunity (assuming a
change in spec which generally makes this valid).

https://godbolt.org/z/K89jdd473

int f(int * q, int * restrict p, int * restrict s)
{
  int x;
  int *r = &x;
  if (p == q)
    r = s;
  *p = 13;
  *r = 42; // `&(*r)` is based on both `p`, `s`!
  return *p;
}

int main(void)
{
  int q;
  return f(&q, &q, &q);
}

[Bug c/105294] restrict pointer - disagreement with specification

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu.org

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
So for GCC r is based on x and s because we use "value based" based-on, that
the value of r changes when p changes shall not mean that this influences
aliasing.  So I agree that the C spec should change (changing 'p' does not
change the set of values 'r' can receive).

Note that GCC, when seeing that r can either be &x (not "restrict") or s
(restrict), makes '*r' effectively not covered by restrict so it relies
on the restrictness of 'p' to disambiguate.  We'd have to assign
restrict tags to each local variable to make it cover this case because
of the way we represent restrict.

I'd have to study llvm to see if they are doing sth more clever here.

[Bug c/105294] restrict pointer - disagreement with specification

[mattlloydhouse at gmail dot com]

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

Matthew House <mattlloydhouse at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mattlloydhouse at gmail dot com

--- Comment #2 from Matthew House <mattlloydhouse at gmail dot com> ---
Isn't this example invalid per the spec? At `*r = 42;`, `&*r` is based on `s`.
However, `*p = 13;` modifies the same object that `*r` refers to, and `&*p` is
not based on `s`, so the behavior is undefined.

I've recently found another example which is valid under the current spec as
well as the two new proposed definitions in N3025 and N3058. 

https://godbolt.org/z/ezvMfPd78

static int x;

__attribute__ ((noinline))
int f(int * restrict p) {
  *p = 1;
  if (p == &x) {
    *p = 2;
  }
  return *p;
}

int main(void) {
  return f(&x);
}

At `*p = 2;`, `&*p` is based on `p` under every definition. However, both GCC
and Clang incorrectly assume that the write to `*p` inside the `if` block
cannot affect the return value.

[Bug tree-optimization/105294] restrict pointer - disagreement with specification

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c                           |tree-optimization
   Last reconfirmed|                            |2023-01-09
             Status|UNCONFIRMED                 |NEW
           Keywords|                            |wrong-code
     Ever confirmed|0                           |1
             Blocks|                            |49774

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Matthew House from comment #2)
> Isn't this example invalid per the spec? At `*r = 42;`, `&*r` is based on
> `s`. However, `*p = 13;` modifies the same object that `*r` refers to, and
> `&*p` is not based on `s`, so the behavior is undefined.
> 
> I've recently found another example which is valid under the current spec as
> well as the two new proposed definitions in N3025 and N3058. 
> 
> https://godbolt.org/z/ezvMfPd78
> 
> static int x;
> 
> __attribute__ ((noinline))
> int f(int * restrict p) {
>   *p = 1;
>   if (p == &x) {
>     *p = 2;
>   }
>   return *p;
> }
> 
> int main(void) {
>   return f(&x);
> }
> 
> At `*p = 2;`, `&*p` is based on `p` under every definition. However, both
> GCC and Clang incorrectly assume that the write to `*p` inside the `if`
> block cannot affect the return value.

While the early alias (restrict) pass figures correct base/cliques the
early VRP pass propagates the conditional equivalence and turns the IL
into

__attribute__((noinline))
int f (int * restrict p)
{
  int _6;

  <bb 2> :
  *p_3(D) = 1;
  if (p_3(D) == &x)
    goto <bb 3>; [INV]
  else
    goto <bb 4>; [INV]

  <bb 3> :
  x = 2;

  <bb 4> :
  _6 = *p_3(D);
  return _6;
}

which in turn is "invalid" as to the restrict rules.  So the core issue is
that we apply (a supposedly stricter set of) restrict rules to our internal
IL rather than the language definition to the language specific AST.  But
the propagation of conditional equivalences makes that "stricter" set of
rules not strict enough.

(Propagating) conditional equivalences is bad.

We _might_ get away with disallowing propagation to restrict qualified
uses, but that might in the end also pessimize things.  We could also
refrain from re-computing restrict info after inlining.


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=49774
[Bug 49774] [meta-bug] restrict qualification aliasing issues