[PATCH] Fix promotion of const local arrays to static storage

[Patrick Palka]

On Mon, Aug 18, 2014 at 6:48 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Mon, Aug 18, 2014 at 4:00 AM, Patrick Palka <patrick@parcs.ath.cx> wrote:
>> Hi,
>>
>> The fix for PR38615 indirectly broke the promotion of const local arrays
>> to static storage in many cases.  The commit in question, r143570, made
>> it so that only arrays that don't potentially escape from the scope in
>> which they're defined (i.e. arrays for which TREE_ADDRESSABLE is 0) are
>> candidates for the promotion to static storage.
>>
>> The problem is that both the C and C++ frontends contain ancient code
>> (dating back to 1994) that sets the TREE_ADDRESSABLE bit on arrays
>> indexed by non-constant or out-of-range indices.  As a result, const
>> arrays that are indexed by non-constant or out-of-range values are no
>> longer candidates for promotion to static storage following the fix to
>> PR38615, because their TREE_ADDRESSABLE bit will always be set.
>> Consequently, array promotion is essentially broken for a large class of
>> C and C++ code.  E.g. this simple example is no longer a candidate for
>> promotion:
>>
>>     int
>>     foo (int i)
>>     {
>>       const int x[] = { 1, 2, 3 };
>>       return x[i]; /* non-constant index */
>>     }
>>
>> This patch removes the ancient code that is responsible for dubiously
>> setting the TREE_ADDRESSABLE bit on arrays indexed by non-constant or
>> out-of-range values.  I don't think that this code is necessary or
>> useful anymore.  Bootstrapped and regtested on x86_64-unknown-linux-gnu,
>> OK for trunk?
>
> This looks good to me - indeed TREE_ADDRESSABLE on variable-indexed
> things isn't necessary (and before that it was made sure to re-set it
> before RTL expansion which required it, as update-address-taken would
> have happily removed TREE_ADDRESSABLE anyway).

Thanks Richard.  Unfortunately I lied when I optimistically said that
regression testing passed.  This patch makes causes a single new
regression in gcc.target/i386/pr14289-1.c.  The file looks like:

/* { dg-options "-O0" } */

register int n[2] asm ("ebx");

void
foo (void)
{
  int i = 0;
  a[i] = 0; /* { dg-error "address of" } */
}

Whereas previously the C FE would have reported an error for trying to
set the TREE_ADDRESSABLE bit on a register variable, now an ICE is
triggered during RTL expansion for trying to index into a register
array with a non-constant index.  So I am thinking of explicitly
checking for this scenario and emitting an error.  Do you think such a
check should be done in the frontends or during RTL expansion?  If
done during RTL expansion, the optimizers could have a chance of
turning the non-constant index into a constant one like in this
example.