Re: [PR64164] drop copyrename, integrate into expand

[Kyrill Tkachov <kyrylo.tkachov@arm.com>]

On 17/08/15 03:56, Alexandre Oliva wrote:
> On Aug 16, 2015, Andreas Schwab <schwab@linux-m68k.org> wrote:
>
>> Alexandre Oliva <aoliva@redhat.com> writes:
>>> On Aug 15, 2015, Andreas Schwab <schwab@linux-m68k.org> wrote:
>>>
>>>> FAIL: gcc.target/aarch64/target_attr_crypto_ice_1.c (internal compiler error)
>>>> In file included from
>>>> /opt/gcc/gcc-20150815/gcc/testsuite/gcc.target/aarch64/target_attr_crypto_ice_1.c:4:0:
>>> Are you sure this is a regression introduced by my patch?
>> Yes, it reintroduces the ICE.
> Ugh.  I see this testcase was introduced very recently, so presumably it
> wasn't present in the tree that James Greenhalgh tested and confirmed
> there were no regressions.
>
> The hack in aarch64-builtins.c looks risky IMHO.  Changing the mode of a
> decl after RTL is assigned to it (or to its SSA partitions) seems fishy.
> The assert is doing just what it was supposed to do.  The only surprise
> to me is that it didn't catch this unexpected and unsupported change
> before.
>
> Presumably if we just dropped the assert in expand_expr_real_1, this
> case would work just fine, although the unsignedp bit would be
> meaningless and thus confusing, since the subreg isn't about a
> promotion, but about reflecting the mode change that was made from under
> us.
>
> May I suggest that you guys find (or introduce) other means to change
> the layout and mode of the decl *before* RTL is assigned to the params?

Hmm, if in TARGET_SET_CURRENT_FUNCTION, which is called fairly
early on to set up cfun I do the relaying of the param decls
then it seems to work. Will do some more testing...


> I think this would save us a ton of trouble down the road.  Just think
> how much trouble you'd get if the different modes had different calling
> conventions, alignment requirements, valid register assignments, or
> anything that might make coalescing their SSA names with those of other
> variables invalid.
>