[patch] Fix PR48183, NEON ICE in emit-rtl.c:immed_double_const() under -g

[Richard Guenther]

On Thu, Mar 24, 2011 at 11:57 AM, Richard Sandiford
<richard.sandiford@linaro.org> wrote:
> Chung-Lin Tang <cltang@codesourcery.com> writes:
>> PR48183 is a case where ARM NEON instrinsics, under -O -g, produce debug
>> insns that tries to expand OImode (32-byte integer) zero constants, much
>> too large to represent as two HOST_WIDE_INTs; as the internals manual
>> indicates, such large constants are not supported in general, and ICEs
>> on the GET_MODE_BITSIZE(mode) == 2*HOST_BITS_PER_WIDE_INT assertion.
>>
>> This patch allows the cases where the large integer constant is still
>> representable using a single CONST_INT, such as zero(0). Bootstrapped
>> and tested on i686 and x86_64, cross-tested on ARM, all without
>> regressions. Okay for trunk?
>>
>> Thanks,
>> Chung-Lin
>>
>> 2011-03-20  Chung-Lin Tang  <cltang@codesourcery.com>
>>
>>       * emit-rtl.c (immed_double_const): Allow wider than
>>       2*HOST_BITS_PER_WIDE_INT mode constants when they are
>>       representable as a single const_int RTX.
>
> I realise this might be seen as a good expedient fix, but it makes
> me a bit uneasy.  Not a very constructive rationale, sorry.
>
> For this particular case, the problem is that vst2q_s32 and the
> like initialise a union directly:
>
>  union { int32x4x2_t __i; __builtin_neon_oi __o; } __bu = { __b; };
>
> and this gets translated into a zeroing of the whole union followed
> by an assignment to __i:
>
>  __bu = {};
>  __bu.__i = __b;

Btw, this looks like a missed optimization in gimplification.  Worth
a bugreport (or even a fix).  Might be a target but as well, dependent
on how __builtin_neon_oi looks like.  Do you have a complete testcase
that reproduces the above with a cross?

Richard.

> We later optimise away the first assignment, but it still exists
> in the debug info.
>
> Another expedient fix might be to replace these initialisations with:
>
>  union { int32x4x2_t __i; __builtin_neon_oi __o; } __bu;
>  __bu.__i = __b;
>
> so that we never get a zeroing statement.
>
> I realise both "fixes" are papering over the real problem.  What we really
> need is arbitrary-length constant integers, like we already have for vectors.
> But that's going to be a much bigger patch.  It just seems to me that,
> if we're going for a work-around, the arm_neon.h change is neutral,
> while changing immed_double_const feels more risky.
>
> Richard
>