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

Replace shifts and rotations of constant 0 with constant 0 #7398

Merged
merged 1 commit into from
Jul 15, 2024

Conversation

dylanjtuttle
Copy link
Contributor

Since applying a shift or rotation to 0 simply produces 0 again, the optimizer can replace any shift or rotation for which the first child is a constant 0 with that constant 0.

Since applying a shift or rotation to 0 simply produces 0 again,
the optimizer can replace any shift or rotation for which the
first child is a constant 0 with that constant 0

Signed-off-by: Dylan Tuttle <jdylantuttle@gmail.com>
@dylanjtuttle
Copy link
Contributor Author

@hzongaro Could I get a review on this (hopefully) fixed version?

@hzongaro hzongaro self-assigned this Jul 3, 2024
Copy link
Contributor

@hzongaro hzongaro left a comment

Choose a reason for hiding this comment

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

I think your changes look good. Thanks!

@hzongaro
Copy link
Contributor

hzongaro commented Jul 5, 2024

Jenkins build all

@hzongaro
Copy link
Contributor

hzongaro commented Jul 5, 2024

@dylanjtuttle, have you run further internal testing of your changes just to make sure everything is exercised fully? If so, may I ask you to post a link to the internal build?

@dylanjtuttle
Copy link
Contributor Author

I have not, let me start one and get back to you when it finishes!

@dylanjtuttle
Copy link
Contributor Author

An internal build with these changes passes sanity.functional and sanity.openjdk. Is that the sort of confirmation you were looking for?

@hzongaro
Copy link
Contributor

hzongaro commented Jul 9, 2024

Is that the sort of confirmation you were looking for?

Yes, thank you - although the problems that we saw with an earlier iteration seemed to occur most frequently with OpenJ9 JDK 21. It could be that changes in the class libraries presented more opportunities for this optimization. May I ask you to rerun that internal build for JDK 21 on a few different platforms?

@dylanjtuttle
Copy link
Contributor Author

After dealing with some flakiness, I got the results I was hoping for: original first rebuild second rebuild

@hzongaro
Copy link
Contributor

After dealing with some flakiness, I got the results I was hoping for

Thank you! Merging.

@hzongaro hzongaro merged commit 3223d8d into eclipse-omr:master Jul 15, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants