Only devirtualize call for compatible classes #3242
Conversation
Change InvariantArgumentPreexistence to only devirtualize method if the propagated arg info with known object index is compatible with the method class of the callsite. Signed-off-by: Yi Zhang <yizhang@ca.ibm.com>
|
@genie-omr build all |
|
@andrewcraik please review this. |
| // | ||
| if (methodSymbol->isVirtual()) | ||
| devirtualize = true; | ||
| if (methodSymbol->isVirtual() && fixedOrFinalInfoExists) |
There was a problem hiding this comment.
Since we moved this logic down from where it was, one difference seems to be the check here that is methodSymbol->isVirtual() as opposed to the original check that was !isInterface. Are these two essentially equivalent at this point in the code ? i.e. we would not be changing the logic for those cases that were entering this code after checking !isInterface originally ?
There was a problem hiding this comment.
I think they are equivalent, originally the logic was guarded by both methodSymbol->isVirtual() || methodSymbol->isInterface() and if (!isInterface && fixedOrFinalInfoExists) , the !isInterface could have been replaced by methodSymbol->isVirtual().
There was a problem hiding this comment.
So we can only devirtualize virtual methods in general in the context where this code is running - interfaces are not strict enough to allow for devirtualization. I agree the conversion looks equivalent. I think the change looks good.
There was a problem hiding this comment.
Okay thanks for the explanation.
|
Looks safe to me and tests have passed. Merging. |
Change InvariantArgumentPreexistence to only devirtualize method if the
propagated arg info with known object index is compatible with the
method class of the callsite.
Signed-off-by: Yi Zhang yizhang@ca.ibm.com