Fix incorrect usage of TR::Options #2715
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
harryyu1994
changed the title
mpirvu
approved these changes
Jun 28, 2018
compiler/compile/OMRCompilation.cpp
Outdated
| @@ -326,13 +326,13 @@ OMR::Compilation::Compilation( | |||
| // while the PersistentMethodInfo is allocated during codegen creation | |||
| // Access to this list must be performed with assumptionTableMutex in hand | |||
| // | |||
| if (!TR::Options::getCmdLineOptions()->getOption(TR_DisableFastAssumptionReclamation)) | |||
| if (!_options->getOption(TR_DisableFastAssumptionReclamation)) | |||
There was a problem hiding this comment.
The parameter options is the same thing here.
compiler/compile/OMRCompilation.cpp
Outdated
| _metadataAssumptionList = new (m->trPersistentMemory()) TR::SentinelRuntimeAssumption(); | ||
| #endif | ||
|
|
||
| //Random fields must be set before allocating codegen | ||
| _primaryRandom = new (m->trHeapMemory()) TR_RandomGenerator(options.getRandomSeed()); | ||
| _adhocRandom = new (m->trHeapMemory()) TR_RandomGenerator(options.getRandomSeed()); | ||
| _primaryRandom = new (m->trHeapMemory()) TR_RandomGenerator(_options->getRandomSeed()); |
There was a problem hiding this comment.
The code was good as it was because options is the per compilation Options object
There was a problem hiding this comment.
yeah I changed it because I saw many more instances of _options than options so I changed everything to _options to avoid confusion. Do you prefer using options or just want this part of the code unchanged?
There was a problem hiding this comment.
Yes, I would prefer to use the parameter options
harryyu1994
changed the title
- during compilation, the per-method TR::Options should be used instead of the global one - mostly happens at the Optimizer and Code Generation stage - simplify comp()->getOptions()->getOption(TR_XXX) to comp()->getOptions(TR_XXX), they are equivalent - switch _options usage to options in OMRCompilation ctor Signed-off-by: Harry Yu <harryyu1994@gmail.com>
|
Hi @vijaysun-omr, could you help merge this change as well as eclipse-openj9/openj9#2290, I think they are ready to go in, thanks a lot. |
vijaysun-omr
approved these changes
Jul 5, 2018
|
@genie-omr build all |
cathyzhyi
pushed a commit
to cathyzhyi/omr
that referenced
this pull request
Jul 30, 2018
Internal pointers are never actually disabled for <= warm level until recent refactoring in OMR changed it to use the correct API and disbled it for real. Disabling internal pointers for <= warm opt level would hurt performance so we need to enable it. pr: eclipse-omr#2715 Signed-off-by: Yi Zhang <yizhang@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.
Signed-off-by: Harry Yu harryyu1994@gmail.com