8377808: compiler/c2/irTests/TestFloat16ScalarOperations.java is fragile in the presence of special constants#31370
Conversation
|
@eme64 and @jatin-bhateja, may I kindly ask for your review? |
|
👋 Welcome back mhaessig! A progress list of the required criteria for merging this PR into |
|
@mhaessig This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 19 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. ➡️ To integrate this PR with the above commit message to the |
|
The total number of required reviews for this PR has been set to 2 based on the presence of this label: |
Webrevs
|
|
Thank you for taking a look, @sendaoYan! I addressed your comments in the latest commit. |
There was a problem hiding this comment.
Does this missed addScenarios(s0)
There was a problem hiding this comment.
It does, but I also do not see the point of running an IR test with value validation with -Xint. Perhaps @jatin-bhateja remembers from back when he wrote that test?
There was a problem hiding this comment.
I just went ahead and refactored it.
merykitty
left a comment
There was a problem hiding this comment.
Looks good to me, thanks for making the change.
eme64
left a comment
There was a problem hiding this comment.
Looks good, thanks for fixing this!
| Scenario s0 = new Scenario(0, "--add-modules=jdk.incubator.vector", "-Xint"); | ||
| Scenario s1 = new Scenario(1, "--add-modules=jdk.incubator.vector"); | ||
| new TestFramework().addScenarios(s1).start(); | ||
| new TestFramework().addFlags("--add-modules=jdk.incubator.vector").start(); |
There was a problem hiding this comment.
Looks like we actually never ran the Xint path! Thanks for cleaning that up.
|
@jatin-bhateja just tagging you here, this is a test we had designed together, and it has now been flaky multiple times. Just so you are aware ;) |
|
Thank you all for reviewing! /integrate |
|
Going to push as commit 8fad96d.
Your commit was automatically rebased without conflicts. |
TestFlaot16ScalarOperations.javais fragile when it comes to unexpected constant folding. Previously, #28678 addressed constant folding for addition with zero. This PR addresses the optimization of division by constants that are reciprocals of powers of two. This is done by reducing the range further to 0.6 to 0.9 as this excludes those reciprocals, which are<= 0.5. In this test we mainly care about the IR matching, so the further reduction of the input space is fine.Testing:
TestFlaot16ScalarOperations.javaon aarch64Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/31370/head:pull/31370$ git checkout pull/31370Update a local copy of the PR:
$ git checkout pull/31370$ git pull https://git.openjdk.org/jdk.git pull/31370/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 31370View PR using the GUI difftool:
$ git pr show -t 31370Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/31370.diff
Using Webrev
Link to Webrev Comment