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.
