RFR: 8257137: Revise smov and umov in aarch64 assembler [v4]

Hao Sun github.com+16932759+shqking at openjdk.java.net
Tue Mar 2 10:12:51 UTC 2021


On Mon, 1 Mar 2021 10:22:53 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> Hao Sun has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Code style: add spaces between operands
>>   
>>   add spaces between operands
>
> src/hotspot/cpu/aarch64/assembler_aarch64.hpp line 2664:
> 
>> 2662:     assert(T != Q, "invalid register variant");
>> 2663:     f(0, 31), f(T == D ? 1 : 0, 30), f(0b001110000, 29, 21);
>> 2664:     f(((idx << 1) | 1) << (int)T, 20, 16), f(0b001111, 15, 10);
> 
> (T == D ? 1 : 0)``` is just ```(T == D)```, isn't it?

Thanks for your comment.

Yes, (T == D ? 1 : 0) equals to (T == D). However, I prefer the former one because it's more accurate to pass an integer as the first argument of 'f' function, compared to passing a boolean value. Besides, the former one is widely used by the definitions of other instructions.

> src/hotspot/cpu/aarch64/assembler_aarch64.hpp line 2675:
> 
>> 2673:     rf(Vn, 5), rf(Rd, 0);
>> 2674:   }
>> 2675: 
> 
> I don't think the code duplication here is necessary. Please pass the validity test and the expression for Bit 30 into ```INSN```. It takes a few minutes studying the difference between those two in order to discover what the differences between them are, which could be saved if we made it explicit.

Thanks for spotting that. Agree. Updated.

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

PR: https://git.openjdk.java.net/jdk/pull/1586


More information about the hotspot-compiler-dev mailing list