RFR: 8261671: X86 I2L conversion can be skipped for certain masked positive values [v9]

Marcus G K Williams github.com+168222+mgkwill at openjdk.java.net
Tue Mar 16 20:48:28 UTC 2021


On Tue, 16 Mar 2021 18:07:48 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

>> Marcus G K Williams has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Refactor BMI2.java based on review
>>   
>>   Signed-off-by: Marcus G K Williams <marcus.williams at intel.com>
>
> src/hotspot/cpu/x86/x86_64.ad line 3179:
> 
>> 3177: %}
>> 3178: 
>> 3179: // Int Immediate: power of 2, positive
> 
> The comment is misleading: it's not a 2^n, but 2^n-1.

�� - Agreed, this is more descriptive.

> src/hotspot/cpu/x86/x86_64.ad line 3183:
> 
>> 3181: %{
>> 3182:   predicate((n->get_int() != 0)
>> 3183:             && ((n->get_int() & 0xc0000000) == 0)
> 
> Why do you use `& 0xc0000000` and not `& 0x80000000` (or even `n->get_int() > 0`)?

Yes. I believe `n->get_int() > 0` would work and be simpler to read.

> src/hotspot/cpu/x86/x86_64.ad line 9178:
> 
>> 9176:   format %{ "bzhiq $dst, $src, $mask \t# using $tmp as TEMP, int & immI_bitmask -> long" %}
>> 9177:   ins_encode %{
>> 9178:     assert(VM_Version::supports_bmi2(), "required");
> 
> The assert is redundant here. The predicate already guarantees that invariant.

Agreed.

> src/hotspot/cpu/x86/x86_64.ad line 3180:
> 
>> 3178: 
>> 3179: // Int Immediate: power of 2, positive
>> 3180: operand immI_bitmask()
> 
> `immI_bitmask` doesn't say much. What about `immI_Pow2M1`?

�� - Agreed, this is more descriptive.

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

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


More information about the hotspot-compiler-dev mailing list