Ping! Saw this again this week in testing. Original test case still failing.

Ping! Saw this again this week in testing. Original test case still failing.

Thanks Jonas for the reminder. I’ll try to look at it this week.

I can reproduce the problem, looking.

At first glance, I'd say the problem is that we have a subrange that is dead, but we fail to see that and thus try to extend the live-range to the next segment, which, since it is dead, does not exist.

The problematic sequence is this one:
%5:gr64bit = LGHI 25
%5.subreg_l32:gr64bit = MSFI %5.subreg_l32:gr64bit(tied-def 0), -117440512

Here subreg_h32 is never used, yet the (sub)live-range is not mark as dead:
%5 L00000001: [160r,176r:0) 0@160r

Thus, when we try to update 176r to the new index, we fail to see that 176r was the last use and look for the next segment.

For the record the full live-interval for %5 at the time of the assert looks like this:
main range: [160r,240r:0)[240r,264B:1)[288B,432r:1)[432r,448r:2)[576B,640B:1) 0@160r 1@240r 2@432r
L00000008: [232r,240r:0)[240r,264B:1)[288B,432r:1)[432r,448r:2)[576B,640B:1) 0@232r 1@240r 2@432r
L00000001: [160r,176r:0) 0@160r weight:0.000000e+00

Notice how L00000001 goes from 160r to 176r, whereas at this point, there is no instruction at 176r. This happens because L00000001 is actually not used by what was at 176r and thus doesn't get updated when we move it around.

Anyway, that's a side problem in the sense that if that lane was marked as dead to beginning with, the use index wouldn't be here, so no update would be required.

Given those live-ranges are created during register coalescing, I am going to look at why we don't mark that lane as dead.

Tentative fix
Tentative patch attached.
Basically, fix the coalescer to properly mark the unused lanes as dead.

Note to myself: would be better if I can mark the dead lanes in refineSubRanges.

I'll go with the tentative fix I've already attached.
Doing the right thing in LiveInterval::refineSubRanges is problematic for two reasons:

1. It may incur in increase in compile time that most users of this function don't need.
2. The IR needs to reflect what the live intervals are supposed to represent, but the register coalescer calls that function before it updates the IR.

We could reuse LiveIntervals::shrinkToUses as well after the IR has been updated, but right now, the rematerialization doesn't call that function so, same problem: potential increase in compile time. Given we know specifically what needs to happen for the remat case, that compile time increase is not really worth it.

Should be fixed with:
To https://github.com/llvm/llvm-project.git
1f822f2..2ec71ea 2ec71ea -> master

@​qcolombet:

Thank you for fixing this :-)

I see that #34922 now also seems fixed ("Live segment doesn't end at a valid instruction"). Do you believe your fix also handles this assertion? I saw a new test this week which also went away with your latest patch included...

@​qcolombet:

Thank you for fixing this :-)

I see that #34922 now also seems fixed
("Live segment doesn't end at a valid instruction"). Do you believe your fix
also handles this assertion? I saw a new test this week which also went away
with your latest patch included...

Yeah, that would make sense that this patch fixes a “doesn’t end at a valid instruction“ assert.

Happy to help!

Thanks for your patience.