[jdk20] RFR: 8299733: AArch64: "unexpected literal addressing mode" assertion failure with -XX:+PrintC1Statistics [v3]

[Kim Barrett]

On Wed, 11 Jan 2023 10:01:35 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> Coming late to the party. Sorry I didn't notice this earlier.
>> 
>> When working on 8297830 I noticed that `uses()` had been problematic for
>> various modes, due to possibly uninitialized members.  I assumed it wasn't
>> being used for such since I didn't find any relevant bug reports. And my
>> changes added asserts that would fire if I was wrong about that, and I didn't
>> see any in fairly broad testing.
>> 
>> The structure of the unionization done by 8297830 makes it easy to add the
>> additional loosening to support literals in `uses()`. For non-literal modes
>> that don't have an index register, `index()` is always initialized to `noreg`.
>> For `no_mode`, `base()` and `index()` are initialized to `noreg` by design, to
>> make these sorts of things easy and not needing to explicitly check for that
>> uncommon case. So no switch statement is needed, just a test for (non)literal.
>> So the following is sufficient:
>> 
>> 
>> bool uses(Register reg) const {
>>   return _mode != literal && (base() == reg || index == reg);
>> }
>
>> ```
>> bool uses(Register reg) const {
>>   return _mode != literal && (base() == reg || index == reg);
>> }
>> ```
> 
> Mmm, but that's fragile under change too, e.g. if we get another literal type. I guess you can argue that's never likely to happen, but I don't think it's appropriate to choose a less robust solution now.

If a new literal type were to show up, there are many places that would need to be examined and probably
updated, including here because of the explicit dispatch on all modes.  So I don't think what I'm suggesting
is any less robust than the current dispatch.  (Note that any change here should be for 21 - my suggested
change doesn't make the cut for 20 at this point.)

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

PR: https://git.openjdk.org/jdk20/pull/87