openj9
openj9 copied to clipboard
eclipse-openj9
Add classes for building IDT and performing abstract interpretation.
Issue: https://github.com/eclipse-openj9/openj9/issues/10485 This is the phase 3/3 of the BenefitInliner contribution.
~~Note: This PR can be merged only after the contribution of BenefitInliner in OMR (https://github.com/eclipse/omr/pull/5509) is merged.~~ The OMR part has been successfully merged, this PR is ready to get review now.
Please merge this PR after merging https://github.com/eclipse/omr/pull/7268!
~~The boolean value insideLoop is intended to be kept as a parameter in some methods in AbsInterpreter.cpp since we would like to make as few as possible changes to the siganature of the public methods. However, this pull request doesn't include the loop handling feature, and it will be introduced by another new pull request after this pull request is merged.~~
Due to time issue, we merged the changes for handling loop into this PR, and there won't be another additional PR for handling loop.
@jdmpapin Hey Devin, this is the OpenJ9 part of BenefitInliner phase 3 and the OMR part (https://github.com/eclipse/omr/pull/5509) has already been merged, so I think this PR is also ready to get review. Since you are the person who most familiar with our project, would you mind to assign yourself as the reviewer of this PR? Thank you very much in advance!🙂
@jdmpapin Hey Devin, I'm sorry for disturbing you, but I'm afraid you has missed my last comment. I just would like to kindly ask you if you mind to assign yourself as the reviewer of this PR? Thank you very much! 🙂
hey @jdmpapin I was wondering if you had a chance to review this PR?
Hi @jdmpapin, could you please take a look for this PR? Thank you! I think most comments are resolved.
BTW, we've decided to merge the changes for handling loops to this PR since we would like to let all our existing codes merged ASAP. So there won't be another PR for handling loops, and this PR is already able to handle loops.
Also, I mistakenly removed a few changes in our last OMR PR, which is for handling loops. I opened another OMR PR (https://github.com/eclipse/omr/pull/7268) for it, could you please also take a look for it? Please note that OMR PR is a pre-requisition for this PR, so it should be merged before than this PR.
Thank you very much!
Hey @jdmpapin, I'm sorry to ping you again. I just would like to kindly remind you that if you have some time to spare, would you mind providing a review for the changes we've recently implemented? Thank you very much for your work and time!
Hey @jdmpapin, sorry for disturbing you again. Would you mind to resume this PR review recently? Thank you very much!
Hi Mingwei and Karim, so sorry, I haven't been able to get to this. Unfortunately, I think it will probably still be some time before I get back to it again
I've verified that the new revision 643741b is a clean squash and rebase of the preexisting series 994ae3b onto 011aecc. (Thankfully I had already fetched 994ae3b, so I was able to compare.) I don't know why GH said there was a conflict before
I've verified that the new revision 643741b is a clean squash and rebase of the preexisting series 994ae3b onto 011aecc. (Thankfully I had already fetched 994ae3b, so I was able to compare.) I don't know why GH said there was a conflict before
Hi Devin, sorry for that I didn't expect you've reviewed this PR so quickly, so I haven't mentioned this is just a clean squash yet. I will fix the comments soon and submit them in my next force-pushed commit soon.