> You rely on being able to see all FRAME accesses as component refs,
> thus nothing transforms them into just MEM[&FRAME, offset].  That's of
> course something that can be easily "broken" by means of doing
> some pointer arithmetic like (untested, but you get the idea)
> 
> foo()
> {
>   int c[32];
>   int j;
>   bar()
>   {
>     int *p = &c[4];
>     p = p + 1;
>     j = *p;
>   }
>   c[4] = 0;
>   bar();
>   return j;
> }
> 
> this should get you j = MEM[<CHAIN>, 4]; in bar and thus a missing
> component-ref when inlining.

The patch compiles hundreds of thousands of lines of Ada everyday at AdaCore, 
how could such a blatant hole have survived that?

> I dont' think it's easily possible to recover from this in your scheme,
> but it would be straight-forward for SRA (you basically look for the
> base variable FRAME and special-case that completely for
> replacement construction, also constraining accesses).

Well, it's implemented in the 30-line block of code under the comment:

+  /* Deal with remaining MEM_REFs, i.e. those for which the field reference
+     has been replaced with the offset.  */

> Marking the FRAME VAR_DECL looks useful, maybe you can split that out
> of your patch?

Sure.

> As of doing it in SRA what I'd do there is special-case FRAME for both
> candidate consideration (so you get around the addressable issue)
> and replacement generation.

OK, but you need to be able to split the FRAME structure without necessarily 
splitting its aggregate fields.  Is that (easily) doable with current SRA?

> Maybe you can open an enhancement bugreport for this and link
> your patch / testcase to it?

Will do.

-- 
Eric Botcazou

Reply via email to