RFR: 8324123: aarch64: fix prfm literal encoding in assembler [v5]

[Andrew Haley]

On Thu, 25 Jan 2024 08:38:41 GMT, Wang Zhuo <wzhuo at openjdk.org> wrote:

>> Current prfm literal mode encoding in aarch64 assembler is not correct.
>> The prfm_literal instruction requires 31 and 30 bits to be 0x11, while current assembler encodes the two bits to be 0x11, which is a ldr instruction, not prfm.
>> For example, if adding the following code in stubGenerator
>> __ prfm(Address(__ pc()))
>> we get a ldr instruction like
>>    ldr x0, 0x0000ffff83f8539c
>> but it should be a prfm instruction like
>>    prfm pldl1keep, 0x0000ffff8ff8539c
>> 
>> The bug is caused in ld_st2, literal mode, bit 31 and 30 bits are set to (size & 0b01), while for prfm instructions, 31 and 30 bits must be 0b11.
>>   void ld_st2(Register Rt, const Address &adr, int size, int op, int V = 0) {
>>     starti;
>> 
>>     f(V, 26); // general reg?
>>     zrf(Rt, 0);
>> 
>>     // Encoding for literal loads is done here (rather than pushed
>>     // down into Address::encode) because the encoding of this
>>     // instruction is too different from all of the other forms to
>>     // make it worth sharing.
>>     if (adr.getMode() == Address::literal) {
>>       assert(size == 0b10 || size == 0b11, "bad operand size in ldr");
>>       assert(op == 0b01, "literal form can only be used with loads");
>>       f(**size & 0b01, 31, 30**), f(0b011, 29, 27), f(0b00, 25, 24);
>>       int64_t offset = (adr.target() - pc()) >> 2;
>>       sf(offset, 23, 5);
>>       code_section()->relocate(pc(), adr.rspec());
>>       return;
>>     }
>> 
>>     f(size, 31, 30);
>>     f(op, 23, 22); // str
>>     adr.encode(&current_insn);
>>   }
>
> Wang Zhuo has updated the pull request incrementally with one additional commit since the last revision:
> 
>   adding checks in prfm encoding to avoid using pre/post index

Still good.

src/hotspot/cpu/aarch64/assembler_aarch64.cpp line 197:

> 195:   // PRFM does not support pre/post index
> 196:   // Passing Address with pre/post mode to ld_st2 will generate an undefined instruction.
> 197:   // So use guarantee to avoid pre/post mode Address operand

Suggestion:

src/hotspot/cpu/aarch64/assembler_aarch64.cpp line 199:

> 197:   // So use guarantee to avoid pre/post mode Address operand
> 198:   guarantee((mode != Address::pre), "prfm does not support pre index");
> 199:   guarantee((mode != Address::post), "prfm does not support post index");

Suggestion:

  guarantee((mode != Address::pre) && (mode != Address::post), "prfm does not support pre/post indexing");

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

Marked as reviewed by aph (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17482#pullrequestreview-1843213993
PR Review Comment: https://git.openjdk.org/jdk/pull/17482#discussion_r1466073793
PR Review Comment: https://git.openjdk.org/jdk/pull/17482#discussion_r1466073361