On Fri, 29 Apr 2022 21:34:13 GMT, Paul Sandoz <psan...@openjdk.org> wrote:

>> Xiaohong Gong has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Rename the "usePred" to "offsetInRange"
>
> IIUC when the hardware does not support predicated loads then any false 
> `offsetIntRange` value causes the load intrinsic to fail resulting in the 
> fallback, so it would not be materially any different to the current 
> behavior, just more uniformly implemented.
> 
> Why can't the intrinsic support the passing a boolean directly? Is it 
> something to do with constants? If that is not possible I recommend creating 
> named constant values and pass those all the way through rather than 
> converting a boolean to an integer value. Then there is no need for a branch 
> checking `offsetInRange`.
> 
> Might be better to hold off until the JEP is integrated and then update, 
> since this will conflict (`byte[]` and `ByteBuffer` load methods are removed 
> and `MemorySegment` load methods are added). You could prepare for that now 
> by branching off `vectorIntrinsics`.

Thanks for your comments @PaulSandoz !

> IIUC when the hardware does not support predicated loads then any false 
> `offsetIntRange` value causes the load intrinsic to fail resulting in the 
> fallback, so it would not be materially any different to the current 
> behavior, just more uniformly implemented.

Yes, it's true that this patch doesn't have any different to the hardware that 
does not support the predicated loads. It only benefits the predicated feature 
supported systems like ARM SVE and X86 AVX-512.

> Why can't the intrinsic support the passing a boolean directly? Is it 
> something to do with constants? If that is not possible I recommend creating 
> named constant values and pass those all the way through rather than 
> converting a boolean to an integer value. Then there is no need for a branch 
> checking offsetInRange.

Yeah, I agree that it's not good by adding a branch checking for 
`offsetInRange`. But actually I met the constant issue that passing the values 
all the way cannot guarantee the argument a constant in compiler at the compile 
time. Do you have any better idea to fixing this?

> Might be better to hold off until the JEP is integrated and then update, 
> since this will conflict (byte[] and ByteBuffer load methods are removed and 
> MemorySegment load methods are added). You could prepare for that now by 
> branching off vectorIntrinsics.

Agree. Thanks!

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

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

Reply via email to