On Mon, Aug 18, 2014 at 6:48 AM, Richard Biener
<richard.guent...@gmail.com> wrote:
> On Mon, Aug 18, 2014 at 4:00 AM, Patrick Palka <patr...@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.
