On Tue, 16 Dec 2025 17:26:57 GMT, Paul Sandoz <[email protected]> wrote:
>> Eric Fang has updated the pull request with a new target base due to a merge >> or a rebase. The pull request now contains four commits: >> >> - Merge branch 'master' into JDK-8372978-fix-umin-umax-identity >> - Declare two constants for UMIN/UMAX reduction identity values >> - Merge branch 'master' into JDK-8372978-fix-umin-umax-identity >> - 8372978: [VectorAPI] Fix incorrect identity values in UMIN/UMAX reductions >> >> The original implementation of UMIN/UMAX reductions in JDK-8346174 >> used incorrect identity values in the Java implementation and test code. >> >> Problem: >> -------- >> UMIN was using MAX_OR_INF (signed maximum value) as the identity: >> - Byte.MAX_VALUE (127) instead of max unsigned byte (255) >> - Short.MAX_VALUE (32767) instead of max unsigned short (65535) >> - Integer.MAX_VALUE instead of max unsigned int (-1) >> - Long.MAX_VALUE instead of max unsigned long (-1) >> >> UMAX was using MIN_OR_INF (signed minimum value) as the identity: >> - Byte.MIN_VALUE (-128) instead of 0 >> - Short.MIN_VALUE (-32768) instead of 0 >> - Integer.MIN_VALUE instead of 0 >> - Long.MIN_VALUE instead of 0 >> >> This caused incorrect result. For example: >> UMAX([42,42,...,42]) returned 128 instead of 42 >> >> Solution: >> --------- >> Use correct unsigned identity values: >> - UMIN: ($type$)-1 (maximum unsigned value) >> - UMAX: ($type$)0 (minimum unsigned value) >> >> Changes: >> -------- >> - X-Vector.java.template: Fixed identity values in reductionOperations >> - gen-template.sh: Fixed identity values for test code generation >> - templates/Unit-header.template: Updated copyright year to 2025 >> - Regenerated all Vector classes and test files >> >> Testing: >> -------- >> All types (byte/short/int/long) now return correct results in both >> interpreter mode (-Xint) and compiled mode. > > test/jdk/jdk/incubator/vector/templates/Kernel-Reduction-Masked-op-func.template > line 12: > >> 10: $abstractvectortype$ av = >> $abstractvectortype$.fromArray(SPECIES, a, i); >> 11: r[i] = av.reduceLanes(VectorOperators.[[TEST]], vmask); >> 12: ra = [[TEST_OP]](ra, r[i]); > > It's probably ok to merge the two cases, but to be conservative assign to a > local variable rather than reading from the array e.g., > > $type$ v = av.reduceLanes(VectorOperators.[[TEST]], vmask); > r[i] = v; > ra = [[TEST_OP]](ra, v); Hi @PaulSandoz . I've changed to using a temporary variable. By the way, are you worried that the `read-after-write` optimization isn't implemented in some architectures? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/28692#discussion_r2634426258
