Remove vestigal trivial store sinking optimization #3893
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
0xdaryl
requested review from
charliegracie,
Leonardo2718,
mstoodle and
vijaysun-omr
as code owners
|
@genie-omr build all |
|
Un-WIPing as downstream dependencies are merged. |
0xdaryl
changed the title
Leonardo2718
suggested changes
May 24, 2019
compiler/optimizer/SinkStores.cpp
Outdated
| @@ -957,7 +908,7 @@ void TR_SinkStores::lookForSinkableStores() | |||
| store->_commonedLoadsUnderTree && | |||
| store->_commonedLoadsUnderTree->get(killedSymIdx)) | |||
| { | |||
| traceMsg(comp()," store->containsUnsatisfedLoadFromSymbol(%d) = %d so %s temp\n",killedSymIdx,store->containsUnsatisfedLoadFromSymbol(killedSymIdx),store->containsUnsatisfedLoadFromSymbol(killedSymIdx) ? "needs":"doesn't need"); | |||
| traceMsg(comp()," store->containsUnsatisfedLoadFromSymbol(%d) = %d so %s temp\n",killedSymIdx, false, "doesn't need"); | |||
There was a problem hiding this comment.
I'm not sure if this trace message still makes sense, since the only thing that can change is killedSymIdx. Maybe it can be rephrased to something like
traceMsg(comp()," killedSymIdx = %d and temp is not needed\n",killedSymIdx);
This optimization is a remnant of a legacy project consuming OMR and is not useful to OMR nor any of its consuming projects. Remove it and repair some source code formatting. Signed-off-by: Daryl Maier <maier@ca.ibm.com>
This function always returns false now that trivial store sinking has been removed. Signed-off-by: Daryl Maier <maier@ca.ibm.com>
This function always returns false now that trivial store sinking has been removed. Signed-off-by: Daryl Maier <maier@ca.ibm.com>
* initCommonedLoadsList(TR::Node *node, vcount_t visitCount) * killCommonedLoadFromSymbol(int32_t symIdx) Signed-off-by: Daryl Maier <maier@ca.ibm.com>
Removing trivial store sinking made this list obsolete. Signed-off-by: Daryl Maier <maier@ca.ibm.com>
* containsKilledCommonedLoad * containsSatisfiedAndNotKilledCommonedLoad * containsCommonedLoad * getCommonedLoad * satisfyCommonedLoad * areAllCommonedLoadsSatisfied * containsUnsatisfiedLoadFromSymbol Remove fields: * _satisfiedCommonedLoadsCount * _commonedLoadsCount Signed-off-by: Daryl Maier <maier@ca.ibm.com>
No longer called once trivial store sinking is removed. Signed-off-by: Daryl Maier <maier@ca.ibm.com>
|
@genie-omr build all |
|
@Leonardo2718 , I think this is ready to go. |
0xdaryl
added a commit
to 0xdaryl/omr
that referenced
this pull request
Sep 17, 2020
The previous cleanup deprecated store sinking code in eclipse-omr#3893 left some unreachable remnants behind. Remove them now. Signed-off-by: Daryl Maier <maier@ca.ibm.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The work to remove is spread across multiple commits as the logic to remove and fold dead code is easier to reason about.