Re: [PATCH 01/10] Fix IRA ICE.

[Richard Sandiford <richard dot sandiford at arm dot com>]

Andrew Stubbs <ams@codesourcery.com> writes:
> On 21/11/2018 00:47, Jeff Law wrote:
>> This seems like a really gross hack and sets an expectation that
>> generating registers in the target after IRA has started is OK.  It is
>> not OK.  THe fact that this works is, IMHO, likely an accident.
>
> What's the proper test for this? Neither lra_in_progress nor 
> reload_in_progress is set here, and can_create_pseudos returns true.
>
> The patterns have the ability to not generate registers, but they don't 
> know not to.
>
> Richard Sandiford has stated that it should be OK, but perhaps the other 
> architectures also work by accident?

Yeah, my understanding was that targets could create new registers here,
and I thought targets did in some situations.  But that's also why I'm
suspicious of the patch.  If GCN is doing something valid and IRA isn't
coping, then the patch seems to be fixing the problem in the wrong place.

So I still agree with Jeff on the gross hack thing...

Richard

> In fact, since we're using LRA (not reload), my understanding is that I 
> ought to be able to create new pseudos right up until reload_completed. 
> (Although, my experience is that it's easy to get into an infinite loop 
> doing that.)
>
>> I think this comes back to the fundamental representational issue with
>> the EXEC handling that still needs to be addressed.
>
> Undoubtedly, this makes it worse, but even without that I'd still want 
> to expand vector memory moves long before split1, so at least some cases 
> have to generate additional registers. (Perhaps IRA doesn't create 
> memory moves though? I'm not sure.)
>
> I'm going to investigate how easy it is to fix the EXEC representation 
> issues. I've been resisting because I had a deadline to make, and it's 
> bound to be an invasive and destabilizing alteration (albeit largely 
> mechanical), but if it's going to be a barrier to commit then probably 
> it's become time. :-(
>
> Andrew