RFR: 8319822: Use a linear-time algorithm for assert_different_registers() [v6]

[Emanuel Peter]

On Mon, 27 Nov 2023 12:57:31 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> At the present time, `assert_different_registers()` uses an O(N**2) algorithm in assert_different_registers(). We can utilize RegSet to do it in O(N) time. This would be a useful optimization for all builds with assertions enabled.
>> 
>> In addition, it would be useful to be able to static_assert different registers. 
>> 
>> Also, I've taken the opportunity to expand the maximum size of a RegSet to 64 on 64-bit platforms.
>> 
>> I also fixed a bug: sometimes `noreg` is passed to `assert_different_registers()`, but it may only be passed once or a spurious assertion is triggered.
>
> Andrew Haley has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Review feedback.

Just a code-style review.

Question: could there be some sort of regression test for this, with different examples and edge cases?

src/hotspot/share/asm/register.hpp line 258:

> 256: template<typename R, typename... Rx>
> 257: inline constexpr bool different_registers(AbstractRegSet<R> allocated_regs, R first_register) {
> 258:   return ! allocated_regs.contains(first_register);

Suggestion:

  return !allocated_regs.contains(first_register);

src/hotspot/share/asm/register.hpp line 264:

> 262: inline constexpr bool different_registers(AbstractRegSet<R> allocated_regs, R first_register, Rx... more_registers) {
> 263:   if (allocated_regs.contains(first_register))
> 264:     return false;

Use curly scope brackets ;)

src/hotspot/share/asm/register.hpp line 286:

> 284: inline void assert_different_registers(R first_register, Rx... more_registers) {
> 285: #ifdef ASSERT
> 286:   if (! different_registers(first_register, more_registers...)) {

Suggestion:

  if (!different_registers(first_register, more_registers...)) {

src/hotspot/share/asm/register.hpp line 291:

> 289:     for (size_t i = 0; i < ARRAY_SIZE(regs) - 1; ++i) {
> 290:       for (size_t j = i + 1; j < ARRAY_SIZE(regs); ++j) {
> 291:         assert(! regs[i]->is_valid() || regs[i] != regs[j],

Suggestion:

        assert(!regs[i]->is_valid() || regs[i] != regs[j],

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

Changes requested by epeter (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16617#pullrequestreview-1824483630
PR Review Comment: https://git.openjdk.org/jdk/pull/16617#discussion_r1453789805
PR Review Comment: https://git.openjdk.org/jdk/pull/16617#discussion_r1453790310
PR Review Comment: https://git.openjdk.org/jdk/pull/16617#discussion_r1453791120
PR Review Comment: https://git.openjdk.org/jdk/pull/16617#discussion_r1453790965