Support Java Behaviour w.r.t Math.max and Math.min for Floating Points - #20185
Conversation
f7231c3 to
0e9063c
Compare
|
jenkins test sanity zlinux jdk11 |
| } else { | ||
| // Normal number (non NaN) | ||
| farg1 = r.nextFloat() * 1000000.0f + 1.0f; | ||
| message = message + "(" + String.valueOf(farg1) + "f, "; |
There was a problem hiding this comment.
You forgot to clean this one - this has caused the functional tests I launched fail. Can you fix this and launch the sanitytests on internal jenkins and share link on slack with me? Once they finish, I will launch test again and then merge.
e54e704 to
63ee962
Compare
Add functional tests for `Math.max` and `Math.min`: - to test specific float and double corner cases - test with NaN, +0, & -0, values to confirm respective omr changes - test all possible execution paths Signed-off-by: Matthew Hall <matthew.hall3@outlook.com>
- Adds java_lang_Math_max/min_float/double as a recognized method - Adds a SupportsInlineMath_MaxMin_FD flag to the Z code generator - Flag is only set in Z if the TR_disableInlineMath_MaxMin_FD environment variable is not set - If the flag is set, call nodes are transformed to a functionally equivalent tree that uses fmin/fmax/dmin/dmax nodes Signed-off-by: Sarwat Shaheen <sarwat.shaheen@yahoo.com>
- spearate evaluators for J9 vs OMR to support differing behaviour (OMR complies with IEEE_754, while J9 returns the first NaN (if present) - +0.0 compares as strictly greater than -0.0 Signed-off-by: Matthew Hall <matthew.hall3@outlook.com>
63ee962 to
bc2c3cf
Compare
|
jenkins test sanity zlinux jdk11 |
|
jenkins test sanity zlinux,xlinux jdk21 |
|
@matthewhall2 Can you triage the failure seen in https://openj9-jenkins.osuosl.org/job/Test_openjdk21_j9_sanity.openjdk_x86-64_linux_Personal/52/ and see if it is related to this change or not? You can run grinder internally with just failing test. |
|
While @matthewhall2 is looking into the failures through, internal jenkins, I have launched a grinder here : https://openj9-jenkins.osuosl.org/job/Grinder/3923 to see how it failed. Going through JIT change once again, I do not think this would cause issue on x86. |
|
internal Grinder build passed @r30shah view/Test_grinder/job/Grinder/44484/ |
|
I see most of the iterations in grinder I launched passing as well. Given that grinders are passing, and the JIT changes will not impact x86, relaunching the sanity.openjdk test on x86. |
|
jenkins test sanity.openjdk xlinux jdk21 |
|
@matthewhall2 Seems like it is consistently failing on this machine, and I picked up last nightly and ran grinder - https://openj9-jenkins.osuosl.org/job/Grinder/3928/console which passes. May be some odd issue. Can you take a look? I Would recommend you to open up issue on runtimes/infrstructure to get access to this system |
|
I think the x86 failures are due to issue #20435 |
Okk, so in that case, I think this one is good to merge. @matthewhall2 please disregard my comment #20185 (comment). Merging this one now. |
|
This is causing a lot of failures in the internal build. I'm going to revert it. |
|
Reverted via #20452 |
|
@matthewhall2 We did initially tested your change so not sure what went wrong. Can you take a look ? |
|
See also the alinux failures found in PR testing today, which seem related. |
6 participants
Enables inlining of Math.max/Math.min functions for floats and doubles
Implements Java standard found while testing
Depends on OMR PR:Support IEEE-754 for fmin/fmax/dmin/dmax nodes eclipse-omr/omr#7464
https://hyc-runtimes-jenkins.swg-devops.com/job/jvm.29.personal/34248/