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: [RFC PATCH] restrict_based_on_field attribute


On Tue, Oct 04, 2011 at 01:55:27PM +0200, Michael Matz wrote:
> Ugh, that's stretching our fragile to unsafe restrict support quite much.  
> IMO beyond what we can reasonably be confident to not cause 
> miscompilations.  Essentially what you're implicitely assuming is that the 
> sources of restrict tags can ultimately be memory, and that this is 
> somehow reliable.  It unfortunately isn't.

If it causes miscompilations, then GCC is buggy and needs to be fixed.
restrict structure fields are not some kind of extension, they are in ISO
C99, see 6.7.3/12).

> Keep in mind that the whole current restrict-tag support was designed to 
> support fortrans specific needs where we can rely on the compiler 
> generating code we most probably (!) won't miscompile later.  Further we 
> interpret some of the part where ISO-C says nothing in our favor.  
> Together this means that e.g. (intentionally _not_ ISO-C, but sort of 
> middle-end speak):
> 
> foo (struct s)
> { restrict int *p1 = s.p;
>   restrict int *p2 = s.p;
>   *p1 = 0; *p2 = 1; return *p1;
> }

Not sure what you mean here.  restrict int doesn't make sense,
int *restrict p1 would make sense, but then it would be invalid C.

> So, loads generate new tags, and we rely on optimizations that this 
> doesn't cause miscompilations via invalid disambiguations, which works 
> fine for the restricted set of cases coming from the fortran frontend.

Loads don't generate new tags.
Try:
struct S { int a; int *__restrict p; };

int *a, *b, *c, *d;

void
foo (S x, S &__restrict y, int z)
{
  if (z > 64)
    {
      a = x.p;
      b = y.p;
      x.p[z] = y.p[z];
    }
  else if (z < 20)
    {
      c = x.p;
      d = y.p;
      y.p[z] = x.p[z];
    }
}

Both the pointers loaded from x.p will have the same heap var in PT info,
similarly both pointers loaded from y.p will have the same heap var in PT
info.

> You introduce even more freedom to this mud.  Effectively:
> 
> foo (struct s)
> { restrict int *p1 = s.p;
>   restrict int *p2 = s.q; // same restrict tag as s.p
> }

Again, in your above syntax it is unclear what is TYPE_RESTRICT, if it is
TYPE_RESTRICT (TREE_TYPE (p1)) or something else.  If the user writes the
above in C and then accesses both through p1 and p2 the same object, it is
invalid.

> Or our own lowering for MEM_EXPR:
> 
> foo (struct s)
> { restrict *p1 = MEM[s, 0];
>   restrict *p2 = MEM[s, 4]; // new tag for p2
> }

Again, if the user writes it this way and accesses through both, it is
invalid.  Additionally, the attribute intentionally doesn't force
TYPE_RESTRICT anywhere (and complains if the field is TYPE_RESTRICT),
so all you'll have is
int *p1 = MEM[s, 0];
int *p2 = MEM[s, 4];
and just PTA will say that both p1 and p2 include the same restrict tag
in the PT set.

I don't see how my patch could make things worse then they already are.

	Jakub


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