-
Notifications
You must be signed in to change notification settings - Fork 396
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
Remove deprecated specialized epilogues #2066
Conversation
* Remove specialized epilogue code and fold surrounding code as necessary Issue: eclipse-omr#1738 Signed-off-by: Daryl Maier <maier@ca.ibm.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IBM Z changes LGTM.
Curious since it seems related, @0xdaryl are we keeping the shrink wrapping code around as well or is that slated for future deprecation? IIRC there were still functional issues with it and it never showed any real performance benefit. |
P changes LGTM. |
@fjeremic , the shrink wrapping code was contributed primarily to demonstrate how one could do dataflow over Now that its part of the permanent git history of Eclipse OMR I'm fine with removing it. It will simplify some aspects of the CodeGenerator as well. The removal should be done as a single PR so that future historians can see all the changes that make up shrink wrapping conveniently should they need to. |
I can only think of one (somewhat speculative) reason the shrink wrapping optimization should stay in the OMR codebase. The main reason why the optimization did not have the impact as originally envisaged was that the inliner became quite aggressive in Java and (generally speaking) has a high success rate for small/medium methods on hot paths. One question I cannot answer with certainty is : will the inliner be as successful in other languages that OMR is being considered for ? If it won't be as successful in some other language due to constraints unique to that scenario, then it might be more important to optimize prologue/epilogue costs (that shrink wrapping type optimization would be a part of). |
I don't have any evidence to say whether it would be more beneficial in an inlining-constrained environment or not. The inliner technology in OMR is somewhat immature (the bulk of the smarts live in OpenJ9 and are highly tuned for Java) which means it may not be as effective or aggressive as Java. This may expose more opportunities @vijaysun-omr postulates. I think the only way we'll know for sure is to dust the code off and make it available for non-Java runtimes. I think the code as-written is reasonably general, but as we've seen with most pieces of code derived with a Java origin there will be some Java-specific semantics that will need to be generalized. Also, I do recall there were functional issues on at least Power that need investigation. I think the shrink wrapping discussion should move to a separate issue (I'll create one). The fate of shrink wrapping in OMR is independent of this PR. |
Fair enough, I will merge this PR independently of the broader shrink wrapping discussion. |
Closes: #1738
Signed-off-by: Daryl Maier maier@ca.ibm.com