RFR: 8289046: Undefined Behaviour in x86 class Assembler [v2]

[Andrew Haley]

On Thu, 23 Jun 2022 15:17:22 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Andrew Haley has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Update src/hotspot/cpu/x86/register_x86.hpp
>>    
>>    Co-authored-by: Aleksey Shipilëv <shade at redhat.com>
>>  - Update src/hotspot/cpu/x86/register_x86.hpp
>>    
>>    Co-authored-by: Aleksey Shipilëv <shade at redhat.com>
>
> src/hotspot/cpu/x86/globalDefinitions_x86.hpp line 78:
> 
>> 76: #endif
>> 77: 
>> 78: #define USE_POINTERS_TO_REGISTER_IMPL_ARRAY
> 
> So, what's the use for this symbol? I see AArch64 code conditionalize definition macros, but this patch does not have such conditionalization. Should it?

It means we pick up the correct definitions of `CONSTANT_REGISTER_DECLARATION` et al.

> src/hotspot/cpu/x86/register_x86.hpp line 171:
> 
>> 169:   // accessors
>> 170:   int raw_encoding() const                       { return this - first(); }
>> 171:   int   encoding() const                          { assert(is_valid(), "invalid register (%d)", (int)raw_encoding() ); return raw_encoding(); }
> 
> In the other cases, we don't do this kind of printing assert. Stick to one style? I think it is fine to have a shorter assert, because `!is_valid()` basically implies `noreg`?

OK.

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

PR: https://git.openjdk.org/jdk/pull/9261