RFR: 8339507: Test generation tool and gtest for testing APX encoding of extended gpr instructions [v4]

[hanklo6]

On Thu, 17 Oct 2024 12:04:55 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> hanklo6 has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 16 additional commits since the last revision:
>> 
>>  - Merge branch 'master' of https://git.openjdk.java.net/jdk into apx-test-tool
>>  - Add comment and defined
>>  - Add copyright header
>>  - Remove tab
>>  - Remove whitespace
>>  - Replace whitespace with tab
>>  - Add flag before testing
>>  - Fix assertion error on MacOS
>>  - Add _LP64 flag
>>  - Add missing header
>>  - ... and 6 more: https://git.openjdk.org/jdk/compare/a9a1795a...ca48f240
>
> test/hotspot/gtest/x86/test_assemblerx86.cpp line 44:
> 
>> 42:   // Different encoding for GCC and OpenJDK
>> 43:   {"shll", {'\xd3', '\xd1'}},
>> 44:   {"shlq", {'\xd3', '\xd1'}},
> 
> For the record.
> 
> // For single register operand salq, C2 assumes shift will be passed through CL register and emits the encoding with opcode set to  'D3".
> void Assembler::shlq(Register dst) {
>   int encode = prefixq_and_encode(dst->encoding());
>   emit_int16((unsigned char)0xD3, (0xE0 | encode));
> }
> 
> // With immediate shift operand we explicitly handle special case of shift by '1' bit... and emit D1 opcode.
> void Assembler::shlq(Register dst, int imm8) {
>   assert(isShiftCount(imm8 >> 1), "illegal shift count");
>   int encode = prefixq_and_encode(dst->encoding());
>   if (imm8 == 1) {
>     emit_int16((unsigned char)0xD1, (0xE0 | encode));
>   } else {
>     emit_int24((unsigned char)0xC1, (0xE0 | encode), imm8);
>   }
> }
> 
> So, GCC toolchain is following a different convention than C2,  but both are emitting correct encodings.
> Our test infrastructure is biased toward C2 and hence does not comply with GCC encoding, thus we are
> skipping over following cases. Please see below a small inline assembly snippet and its corresponding encoding.
> 
> void micro(){
>    asm volatile(
>       "shlq $1, %%r11          \n\t"                                [InstID 1]
>       "shlq %%r11              \n\t"                                [InstID 2]
>       "shlq %%cl, %%r11        \n\t"                                [InstID 3]
>   :  :  : "%r11", "%rcx"
>    );
> }
> CPROMPT>objdump -D shl.o
> Disassembly of section .text:
> 0000000000000000 <micro>:
>    0:   55                      push   %rbp
>    1:   48 89 e5                mov    %rsp,%rbp
>    4:   49 d1 e3                shl    $1,%r11                       [InstID 1]
>    7:   49 d1 e3                shl    $1,%r11                       [InstID 2]                
>    a:   49 d3 e3                shl    %cl,%r1 1                     [InstID 3]

I think I can put the `cl` register in the GCC assembly to align it with the JDK assembler. This will allow us to remove these parts of checking.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20857#discussion_r1805590978