On Tue, 3 Sep 2024 22:18:20 GMT, Sandhya Viswanathan <sviswanat...@openjdk.org>
wrote:
>> Jatin Bhateja has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review comments resolved
>
> src/hotspot/cpu/x86/x86.ad line 10684:
>
>> 10682: match(Set dst (SaturatingSubVI src1 src2));
>> 10683: match(Set dst (SaturatingSubVL src1 src2));
>> 10684: effect(TEMP xtmp1, TEMP xtmp2);
>
> Here we need TEMP dst in effect, the saturating_unsigned_sub_dq_avx defines
> and uses dst across xtmp1.
Thanks, yes live range of MachNode corresponding to TEMP ends at its consumer
instruction, they never make their way into liveout set of its block or survive
beyond consumer, but back to back updates to DST and TMP may corrupt DST if
both are assigned same registers by allocator.
> src/java.base/share/classes/java/lang/Long.java line 1987:
>
>> 1985: public static long addSaturating(long a, long b) {
>> 1986: long res = a + b;
>> 1987: // HD 2-12 Overflow iff both arguments have the opposite sign
>> of the result
>
> HD -> Hacker's Delight
Thanks for elaborating, I replicated this logic from
https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/lang/Math.java#L930
Wanted to comply with rest of the codes.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20507#discussion_r1744970392
PR Review Comment: https://git.openjdk.org/jdk/pull/20507#discussion_r1744970574
