Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Tree Simplifier convertCurrentTimeMillis() #18312

Merged

Conversation

klangman
Copy link
Contributor

The convertCurrentTimeMillis() function was recreating a currentTimeMillis() call into a new call target which would be replaced in codegen with a Z specific instruction. To convert the results into milliseconds the value has to be divided by a constant and the tree was incorrectly converted into an ldiv with a call child that is not anchored which allowed dead trees elimination to remove the treetop resulting in the the ldiv to be moved in a way that would change the semantic of the java method.

The convertCurrentTimeMillis() function was recreating a
currentTimeMillis() call into a new call target which would be replaced
in codegen with a Z specific instruction. To convert the results into
milliseconds the value has to be divided by a constant and the tree
was incorrectly converted into an ldiv with a call child that is not
anchored which allowed dead trees elimination to remove the treetop
resulting in the the ldiv to be moved in a way that would change the
semantic of the java method.

Signed-off-by: Kevin Langman <langman@ca.ibm.com>
@klangman
Copy link
Contributor Author

@vijaysun-omr

@vijaysun-omr
Copy link
Contributor

Jenkins test sanity all jdk11

@pshipton
Copy link
Member

@vijaysun-omr the machines at unb aren't usable atm, see the announcement on Slack
https://openj9.slack.com/archives/C8PQL5N65/p1697653111210999
This testing may take down jenkins and/or cause many jobs to fail, including in the nightly builds.

@vijaysun-omr
Copy link
Contributor

jenkins test sanity zlinuxjit jdk21

@vijaysun-omr
Copy link
Contributor

Checks have passed except on aix that does not look like a real issue. This is a cross platform change anyway that looks safe to me. Merging.

@vijaysun-omr vijaysun-omr merged commit 869824b into eclipse-openj9:master Oct 27, 2023
20 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants