RFR: 8325467: Support methods with many arguments in C2 [v11]

[Daniel Lundén]

On Mon, 31 Mar 2025 13:24:09 GMT, Roberto Castañeda Lozano <rcastanedalo at openjdk.org> wrote:

>> Daniel Lundén has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Extend example with offset register mask
>
> src/hotspot/share/opto/postaloc.cpp line 686:
> 
>> 684:               assert(!(!value[ureg_lo] && lrgs(useidx).mask().is_offset() &&
>> 685:                        !lrgs(useidx).mask().Member(ureg_lo)),
>> 686:                      "invalid assumption");
> 
> Could you use more descriptive names and assertion messages in this new assertion and the one below? Ideally, without having to refer to old versions. What is the invariant that we want to check? How does it relate to the surrounding code?

As we've previously discussed offline, I also had my doubts when introducing these asserts. I've now had a second look (with reasonably fresh eyes), and believe I now better understand the underlying assumptions.

The two problematic pieces of code in `postaloc.cpp` from before this changeset that we need to translate as part of the changeset are

if (!value[ureg_lo] &&
    (!RegMask::can_represent(ureg_lo) ||
     lrgs(useidx).mask().Member(ureg_lo))) { // Nearly always adjacent

and

if( RegMask::can_represent(nreg_lo) &&     // Either a spill slot, or
    !lrgs(lidx).mask().Member(nreg_lo) ) { // Nearly always adjacent

Specifically, the `RegMask::can_represent` calls check if their argument registers can fit in the statically determined size of register masks (which no longer makes sense in this changeset).

The reason for the `can_represent` calls is that the subsequent `Member` calls assert internally that their arguments can fit within the static size of register masks. That is, `can_represent` worked as a guard to ensure the precondition for the call to `Member` holds. In this changeset, the `Member` function is generalized to allow arbitrary arguments (and the interal assert is removed). Therefore, we can remove the `can_represent` guards.

Now to the assertions that I added (which I've now improved). From the if conditions, we can infer there is an implicit invariant that a register for which `can_represent` returns false is necessarily "adjacent". Specifically, `can_represent` returning false implies that the register is a spill slot (implied by a comment in the source code). However, registers for which `can_represent` returns true may **also** be spill splots, so using `can_represent` as a proxy check for spill slots feels clumsy. I believe that the real invariant here is that only actual registers (and not stack locations, including spill slots) can be non-adjacent. This is what I now verify with my updated asserts.

For the record, I have not been able to find any cases with non-adjacency in any tests on current Oracle-supported platforms. From another comment in the source code, it looks like non-adjacent pairs are quite specific to SPARC.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/20404#discussion_r2026915863