Skip to content

Conversation

@XiaohongGong
Copy link

@XiaohongGong XiaohongGong commented Dec 23, 2025

The test fails intermittently with the following error:

Caused by: java.lang.RuntimeException: assertEqualsWithTolerance: expected 0.0 but was 1.1754945E-38 (tolerance: 1.4E-44, diff: 1.1754945E-38)
at compiler.vectorapi.TestVectorOperationsWithPartialSize.verifyAddReductionFloat(TestVectorOperationsWithPartialSize.java:231)
at compiler.vectorapi.TestVectorOperationsWithPartialSize.testAddReductionFloat(TestVectorOperationsWithPartialSize.java:260)

The root cause is that the Vector API reduceLanes() does not guarantee a specific calculation order for floating-point reduction operations [1]. When the array contains extreme values, this can produce results outside the tolerance range compared to sequential scalar addition.

For example, given array elements:

[0.0f, Float.MIN_NORMAL, Float.MAX_VALUE, -Float.MAX_VALUE]

Sequential scalar addition produces:

0.0f + Float.MIN_NORMAL + Float.MAX_VALUE - Float.MAX_VALUE = 0.0f

However, reduceLanes() might compute:

(0.0f + Float.MIN_NORMAL) + (Float.MAX_VALUE - Float.MAX_VALUE) = Float.MIN_NORMAL

The difference of the two times of calculation is Float.MIN_NORMAL (1.1754945E-38), which exceeds the tolerance of Math.ulp(0.0f) * 10.0f = 1.4E-44. Even with a 10x rounding error factor, the tolerance is insufficient for such edge cases.

Since reduceLanes() does not require a specific calculation order, differences from scalar results can be significantly larger when special or extreme maximum/minimum values are present. Using a fixed tolerance is inappropriate for such corner cases.

This patch fixes the issue by initializing the float array in test with random normal values within a specified range, ensuring the result gap stays within the defined tolerance.

Tested locally on my AArch64 and X86_64 machines 500 times, and I didn't observe the failure again.

[1] https://docs.oracle.com/en/java/javase/25/docs/api/jdk.incubator.vector/jdk/incubator/vector/FloatVector.html#reduceLanes(jdk.incubator.vector.VectorOperators.Associative)


Progress

  • Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue

Issue

  • JDK-8373722: [TESTBUG] compiler/vectorapi/TestVectorOperationsWithPartialSize.java fails intermittently (Bug - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/28960/head:pull/28960
$ git checkout pull/28960

Update a local copy of the PR:
$ git checkout pull/28960
$ git pull https://git.openjdk.org/jdk.git pull/28960/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 28960

View PR using the GUI difftool:
$ git pr show -t 28960

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/28960.diff

Using Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Dec 23, 2025

👋 Welcome back xgong! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk
Copy link

openjdk bot commented Dec 23, 2025

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

@openjdk
Copy link

openjdk bot commented Dec 23, 2025

@XiaohongGong The following label will be automatically applied to this pull request:

  • hotspot-compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

@openjdk openjdk bot added the rfr Pull request is ready for review label Dec 23, 2025
@mlbridge
Copy link

mlbridge bot commented Dec 23, 2025

Webrevs

@DamonFool
Copy link
Member

Shall we change the calculation of tolerance?

e.g.

max_abs = max(abs(arr[0]), abs(arr[1]), ...)
tolerance = Math.ulp(max_abs) * vlen

@XiaohongGong
Copy link
Author

Shall we change the calculation of tolerance?

e.g.

max_abs = max(abs(arr[0]), abs(arr[1]), ...)
tolerance = Math.ulp(max_abs) * vlen

Thanks for looking at this issue. It's really a good question about the definition of a more reasonable tolerance, which is fundamentally a numerical analysis problem per my understanding.

For a floating‑point reduction test, the goal is to check that the API is implemented correctly. And what we really care about is how close the reduction result is to the mathematically expected value. In other words, the tolerance should be derived from the expected sum itself. For example, if the float array is [1.0f, -1.0f, 2.0f, -2.0f], we genuinely expect a result very close to 0.0f, not something near 2.0f.

Using max_abs to set the tolerance risks inflating the admissible error range (since max_abs here would be 2.0f), which I'm afraid might make the test much less effective. WDYT?

@DamonFool
Copy link
Member

And what we really care about is how close the reduction result is to the mathematically expected value.

Understood.

However, (1.0, 5.0) the test range is really too small.
If we expand the range, the current tolerance may still become not big enough.

@DamonFool
Copy link
Member

For example, if the float array is [1.0f, -1.0f, 2.0f, -2.0f], we genuinely expect a result very close to 0.0f, not something near 2.0f.

Using max_abs to set the tolerance risks inflating the admissible error range (since max_abs here would be 2.0f), which I'm afraid might make the test much less effective.

FYI: the current sum based tolerance may be also bigger than max_abs based.
For example, if the float array is [1.0f, 1.0f, 2.0f, 2.0f]. The sum would be 6.0f, the max_abs would be 2.0.
What do you think?

@XiaohongGong
Copy link
Author

XiaohongGong commented Dec 24, 2025

For example, if the float array is [1.0f, -1.0f, 2.0f, -2.0f], we genuinely expect a result very close to 0.0f, not something near 2.0f.
Using max_abs to set the tolerance risks inflating the admissible error range (since max_abs here would be 2.0f), which I'm afraid might make the test much less effective.

FYI: the current sum based tolerance may be also bigger than max_abs based. For example, if the float array is [1.0f, 1.0f, 2.0f, 2.0f]. The sum would be 6.0f, the max_abs would be 2.0. What do you think?

We used the Math.ulp(sum) * 10 to calculate the tolerance, which means the difference of expected and actual value is inside of 10 ULP around the value of sum. Note that Math.ulp(f) is the positive distance between f and the next representable float value larger in magnitude (1 ulp around value f). Hence, it is more reasonable based on the expected value?

@XiaohongGong
Copy link
Author

And what we really care about is how close the reduction result is to the mathematically expected value.

Understood.

However, (1.0, 5.0) the test range is really too small. If we expand the range, the current tolerance may still become not big enough.

What I really want to avoid in this case is generating values that are near the extreme maximum or minimum of float. Expanding the value range would probably still be acceptable with the current tolerance, which is derived not only from the cross‑lane sum but also includes a rounding‑error factor of 10.

BTW, consider that there are already enough API tests under test/jdk/jdk/incubator/vector, this test deliberately uses a narrower range, because its primary goal is to verify the expected IR generated on SVE, rather than to stress all numerical edge cases.

@DamonFool
Copy link
Member

We used the Math.ulp(sum) * 10 to calculate the tolerance, which means the difference of expected and actual value is inside of 10 ULP around the value of sum.

The question here is what do you mean by expected value?
Do you mean the sequential scalar floating point add with rounding errors will produce the expected value?
Or do you mean the golden value in math?

Just consider the following case

1000.0f + Float.MAX_VALUE - Float.MAX_VALUE

The sequential scalar floating add will result 0.0f.
However, the golden value in math should be 1000.0f.

@XiaohongGong
Copy link
Author

We used the Math.ulp(sum) * 10 to calculate the tolerance, which means the difference of expected and actual value is inside of 10 ULP around the value of sum.

The question here is what do you mean by expected value? Do you mean the sequential scalar floating point add with rounding errors will produce the expected value? Or do you mean the golden value in math?

Just consider the following case

1000.0f + Float.MAX_VALUE - Float.MAX_VALUE

The sequential scalar floating add will result 0.0f. However, the golden value in math should be 1000.0f.

I think we should refer to the java ref spec, that calculates the values in sequential order, right?

@DamonFool
Copy link
Member

I think we should refer to the java ref spec, that calculates the values in sequential order, right?

I'm not sure. Did you find related description in the spec?

If that is true, the current sum-based Math.ulp(0.0f) * 10 is far from 1000.0f, which seems unreasonable.

Comment on lines +79 to +80
random.fill(random.uniformFloats(1.0f, 5.0f), fa);
random.fill(random.uniformDoubles(1.0, 5.0), da);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally our tolerance window should be narrow, and increasing the tolerance range to accomodate outliers as you mentioned in your issue description may defeat the purpose.

Unlike auto-vectorization which adhears strict ordering JLS semantics, vectorAPI relaxes the reduction order to give backends leeway to use parallel reduction not strictly following the sequential order.

There are multiple considerations involed, fallback implimentation performs reduction sequentially, inline expander always relaxes the strict ordering, intrinsification of Add/Mul reductions are only supported by Aarch64, X86 and riscv.

Computing expected value using parallel reduction can be other alternative but then we may face similar problems on targets which does not intrinsify unordered reductions.

Tolerance modeling is a complex topic and involves relative and absolute error, current 10ULP absolute limit is not generic enough to handle entier spectrum of values, what you have enforced now is a range based tolerance did you try widening the input value range and confirm if 10ULP tolerance limit is sufficient ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hotspot-compiler [email protected] rfr Pull request is ready for review

Development

Successfully merging this pull request may close these issues.

3 participants