RFR: 8344355: Register corruption in MacroAssembler::lookup_secondary_supers_table_var: x86-64 only

[Andrew Haley]

On Mon, 25 Nov 2024 23:28:49 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

>> src/hotspot/cpu/x86/macroAssembler_x86.cpp line 4915:
>> 
>>> 4913:     bind(done);
>>> 4914:   }
>>> 4915: #ifdef ASSERT
>> 
>> I understand that it greatly increases the reproducibility of the bug, but does such an adhoc code snippet adds much from maintenance perspective? Reusing registers is common in assembly code, so if all places where a register value is destroyed are annotated in a similar way, it would add a lot of noise on source code level.
>
> I'm looking at `MacroAssembler::lookup_secondary_supers_table_var()` and the only thing which can be improved there is an extra comment around line 5116 stressing that `population_count` destroys `slot`.

> I understand that it greatly increases the reproducibility of the bug, but does such an adhoc code snippet adds much from maintenance perspective? Reusing registers is common in assembly code, so if all places where a register value is destroyed are annotated in a similar way, it would add a lot of noise on source code level.

The point isn't so much that it's destroyed, but that it's destroyed on a rarely-used code path, in this case on a code path that is only ever executed on an obsolete machine. We've had the same problem on AArch64, where there are the old and new versions of compare-and-swap, and fixed the testing in the same way.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22365#discussion_r1858229650