Fix incorrect usage of TR::Options #2290
Conversation
|
@mpirvu Hi Marius, FYI the pull request for incorrect usage of TR::Options in optimizer/code generator is here. There is going to be one for OMR as well. |
|
OMR side changes: eclipse-omr/omr#2715 |
| @@ -1116,7 +1116,7 @@ TR_ResolvedJ9MethodBase::isInlineable(TR::Compilation *comp) | |||
|
|
|||
| static intptrj_t getInitialCountForMethod(TR_ResolvedMethod *m, TR::Compilation *comp) | |||
| { | |||
| TR::Options * options = comp->getOptions()->getCmdLineOptions(); | |||
| TR::Options * options = comp->getOptions(); | |||
There was a problem hiding this comment.
I am torn about this one: we are asking about the initial count for method m which is not the same as the method being compiled. It's true that asking for the global options is not correct either because the user could have provided specific options for that method.
There was a problem hiding this comment.
Is that true 100% of the time? Are there cases where method m and method being compiled are the same method?
TR_ResolvedMethod *compMethod = comp->getCurrentMethod();
count = compMethod->getInvocationCount();
initialCount = getInitialCountForMethod(compMethod, comp);
^ this looks like they are the same method, no?
There was a problem hiding this comment.
So this is the call where method m and method being compiled could be different
intptrj_t initialCount = getInitialCountForMethod(this, comp);
is there a way to get options from TR_ResolvedMethod *m.
|
jenkins test all |
|
Please do another "jenkins test all". |
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 Signed-off-by: Harry Yu <harryyu1994@gmail.com>
- comp()->getOptions()->getOption(TR_XXX) is equivalent to comp()->getOptions(TR_XXX) - therefore simplify all similar instances Signed-off-by: Harry Yu <harryyu1994@gmail.com>
|
jenkins test all |
|
It does not look like the win test failures are related to this change. I think this is safe to merge (once the linux_390 test finishes). |
|
Failures are not related to the commit, so I'll merge it |
Signed-off-by: Harry Yu harryyu1994@gmail.com