[committed] patch to fix PR97978

[Vladimir Makarov]

On 2021-01-07 6:01 a.m., Richard Sandiford wrote:
> Vladimir Makarov via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>> The following fixes
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97978
>>
>> The patch was successfully bootstrapped on x86-64.
> Can you explain this a bit more?  The assert fires if the register
> allocation is inconsistent with the conflict information.  What causes
> the inconsistency in this case, and why is it OK for the inconsistency
> to persist until the next lra_assign pass?  Does something fix up the
> inconsistency later, or is the inconsistent information never used?

Sorry, Richard.  I should have described it in more details in the bugzilla.

Hard register splitting was added to remove reload failures to find a 
hard register after the first scheduling.  Although unfortunately it 
does not remove all the 1st insn scheduling problems.

A pseudo (p1) lives through insn and one insn alternative requires a 
specific hard reg (dx in this case) although there is no explicit hard 
register in the insn (simply another pseudo p2 in the insn requires a 
class containing only dx reg).  So the pseudo p1 does not have conflict 
with dx for now and p1 was assigned to dx.

Now constraint sub-pass choose the insn alternative and the next assign 
sub-pass creates a reload pseudo rp2 for p2 and tries to assign dx to 
rp2.  It fails and we made hard reg splitting to assign dx to a reload 
pseudo of rp2.

p3<-dx

insn (rp2)

dx<-p3

After this transformation p1 now got dx as a conflicting reg and 
allocation is incorrect at this stage as p1 assigned to dx living 
through the insns conflicts explicitly with dx.

Next assign subpass is trying to assign dx to rp2 by spilling p1 (and 
assigning another hard reg to p1).

At the end p2,rp2,p3 get dx and all trivial moves (dx <- dx) are removed.

That is explanation when we have an incorrect allocation.

I also should say that LRA has a final register allocation check.  The 
check in assign sub-pass is for earlier bug recognition during initial 
LRA development and probably is not necessary anymore.  So another 
solution would be remove the check in assign sub-pass at all.  But I 
decided not to do this as it still permits to find some bugs earlier.

> Is there no chance of lra_split_hard_reg_for updating the information
> itself, to keep everything self-consistent?
I think it would be the old reload approach (ignoring sub-pass 
separation and making all in on place).  To correct allocations right in 
lra_split_hard_reg_for we would need to check all pseudos living through 
the new insns (and for this we need correct live info) and spill pseudos 
conflicting with split hard reg.  But we don't need to do this as right 
after splitting sub-pass we have live info sub-pass and assign sub-pass 
which make all of this.
>    Bypassing the check for
> every pseudo register seems like quite a big hammer.
>
> I'm not saying this is the wrong fix.  I just think it would help
> to have more commentary explaining the situation.
>