Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refine linkToVirtual to a direct call when no vtable entry exists #13474

Merged
merged 1 commit into from
Sep 13, 2021
Merged

Refine linkToVirtual to a direct call when no vtable entry exists #13474

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 19 additions & 12 deletions runtime/compiler/optimizer/J9TransformUtil.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2180,26 +2180,33 @@ J9::TransformUtil::refineMethodHandleLinkTo(TR::Compilation* comp, TR::TreeTop*
TR::ILOpCodes callOpCode = getTargetMethodCallOpCode(rm, node->getDataType());

TR::SymbolReference* newSymRef = NULL;
uint32_t vTableSlot = 0;
int32_t jitVTableOffset = 0;
if (rm == TR::java_lang_invoke_MethodHandle_linkToVirtual)
{
uint32_t vTableSlot = fej9->vTableOrITableIndexFromMemberName(comp, mnIndex);
auto resolvedMethod = fej9->createResolvedMethodWithVTableSlot(comp->trMemory(), vTableSlot, targetMethod, symRef->getOwningMethod(comp));
newSymRef = comp->getSymRefTab()->findOrCreateMethodSymbol(symRef->getOwningMethodIndex(), -1, resolvedMethod, callKind);
newSymRef->setOffset(fej9->vTableSlotToVirtualCallOffset(vTableSlot));
}
else
{
uint32_t vTableSlot = 0;
auto resolvedMethod = fej9->createResolvedMethodWithVTableSlot(comp->trMemory(), vTableSlot, targetMethod, symRef->getOwningMethod(comp));
newSymRef = comp->getSymRefTab()->findOrCreateMethodSymbol(symRef->getOwningMethodIndex(), -1, resolvedMethod, callKind);
}
vTableSlot = fej9->vTableOrITableIndexFromMemberName(comp, mnIndex);
jitVTableOffset = fej9->vTableSlotToVirtualCallOffset(vTableSlot);
// For private virtual methods and java/lang/Object; type virtual methods, there is no corresponding
// entry in the vtable, and for such methods the interpreter vtable index is 0.
// The dispatch is not performed through the vtable entry, but directly dispatched to the J9Method
// in Membername.vmtarget. In such cases, we can refine a linkToVirtual call similar to linkToStatic.
// jitVTableOffset is obtained using sizeof(J9Class) - interpreter vtable offset, resulting in a positive
// value when the interpreter vtable index we get (from MemberName.vmindex.vtableoffset) is set as 0.
if (jitVTableOffset > 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused by this test. If the problematic interpreter vtable offset is "sizeof(J9Class)" or +376 then shouldn't this test be jitVTableOffset >= 0 ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problematic interpreter vtable offset is when it is set as 0, resulting in the jitVtableOffset being sizeof(J9Class). Normally, interpreter vtable offset is set a value > sizeof(J9Class) so the vtable offset we obtain will always be negative, which is why I thought a simple test for a positive integer value was sufficient. We can't have jitVtableOffset == 0 as that is the start of the J9Class. Do you think changing the operator to >= would provide more clarity?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for reference, here is where the interpreter vtable offset is set:

openj9/runtime/vm/createramclass.cpp

Lines 1407 to 1421 in 7ad8670

/* Iterate over the vtable from the end to the beginning, skipping
* the "magic" header entries. This ensures that the most "recent" override of the
* method is found first. Critically important for super sends
* ie: invokespecial) being correct
*/
J9VTableHeader *vTable = J9VTABLE_HEADER_FROM_RAM_CLASS(methodClass);
J9Method **vTableMethods = J9VTABLE_FROM_HEADER(vTable);
UDATA vTableIndex = vTable->size;
while (vTableIndex > 0) {
vTableIndex -= 1;
if (method == vTableMethods[vTableIndex]) {
return J9VTABLE_OFFSET_FROM_INDEX(vTableIndex);
}
}

J9VTABLE_OFFSET_FROM_INDEX takes the index in the vtable and adds sizeof(J9Class) + sizeof(J9VTableHeader), and the vtable entries begin following the J9VTableHeader. So whenever there's a vtable entry, the jitVTableOffset will end up being a negative value.

callKind = TR::MethodSymbol::Static;
}
auto resolvedMethod = fej9->createResolvedMethodWithVTableSlot(comp->trMemory(), vTableSlot, targetMethod, symRef->getOwningMethod(comp));
newSymRef = comp->getSymRefTab()->findOrCreateMethodSymbol(symRef->getOwningMethodIndex(), -1, resolvedMethod, callKind);
if (callKind == TR::MethodSymbol::Virtual)
newSymRef->setOffset(jitVTableOffset);

bool needNullChk, needVftChild, needResolveChk;
needNullChk = needVftChild = false;
switch (rm)
{
case TR::java_lang_invoke_MethodHandle_linkToVirtual:
needVftChild = true;
if (callKind == TR::MethodSymbol::Virtual)
needVftChild = true;
// fall through
case TR::java_lang_invoke_MethodHandle_linkToSpecial:
needNullChk = true;
Expand Down