[patch] split FRAME variables back into pieces

[Richard Guenther]

On Thu, Sep 20, 2012 at 12:56 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> I really don't like this to be done outside of SRA (and it is written in a
>> non-MEM_REF way).
>
> Could you elaborate on the latter point?  If it can be improved, even in its
> current form...

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.

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).

>> For the testcase in question we scalarize back
>> 'i' in SRA (other scalars are optimized away already, but as SRA runs
>> before DSE it still gets to see stores to FRAME.i).  Now I wonder
>> why we generate reasonable debug info even without inlining,
>> thus there has to be a association to the original decls with
>> the frame FIELD_DECLs.  That is, lookup_decl_for_field should not be
>> necessary and what we use for debug info generation should be used by SRA
>> to assign a name to scalarized fields.
>
> The testcase is a toy example of course.

Yes, I realize that.

>> That alone would not solve your issue because of the 'arr' field in
>> the structure which cannot be scalarized (moved to a stand-alone
>> decl) by SRA.  That's one missed feature of SRA though, and generally
>> useful.
>
> The improved scalarization of aggregates is the main point and what yielded
> the performance boost for SPARKSkein.
>
>> So no, I don't think this patch is the right approach.
>
> OK, but I came to the opposite conclusion when I first tried to do it in SRA
> and I don't think I will change my mind in the near future.  Never mind then.

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

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.

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

Thanks,
Richard.

> --
> Eric Botcazou