Skip to content

Commit

Permalink
Fix object allocation inlining on Power under SVM AOT
Browse files Browse the repository at this point in the history 
Object allocation inlining in the Power codegen now works correctly when
the Symbol Validation Manager is enabled. The previous code was making a
number of incorrect assumptions about what would happen under AOT, which
have now been corrected.

Specifically, when allocating an array, the old AOT infrastructure would
put the address of the component class in classReg and the array class
would be retrieved from that at runtime. Under SVM AOT, it is now
possible to directly load the address of the array class, so classReg
will now contain the address of the array class and the runtime
retrieval of the array class from the component class is no longer
necessary.

Signed-off-by: Ben Thomas <[email protected]>
  • Loading branch information
@aviansie-ben
aviansie-ben committed Feb 21, 2019
1 parent 9a88f0a commit 715b9d3
Showing 1 changed file with 47 additions and 3 deletions.
50 changes: 47 additions & 3 deletions runtime/compiler/p/codegen/J9TreeEvaluator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6381,7 +6381,7 @@ static void genInitObjectHeader(TR::Node *node, TR::Instruction *&iCursor, TR_Op

TR::Register * clzReg = classReg;

if (comp->compileRelocatableCode())
if (comp->compileRelocatableCode() && !comp->getOption(TR_UseSymbolValidationManager))

This comment has been minimized.

Sign in to view

Copy link
@gita-omr

gita-omr Feb 21, 2019

Contributor

Why do we need to skip this code with SVM?

This comment has been minimized.

Sign in to view

Copy link
@aviansie-ben

aviansie-ben Feb 21, 2019

Author Contributor

This is the code for loading the array class given the component class. As I stated in the commit message that you asked me to clarify, this code is not used under SVM AOT because we can simply load the address of the array class directly rather than loading the address of the component class and then using this code to get the array class at runtime.
{
if (node->getOpCodeValue() == TR::newarray)
{
Expand Down Expand Up @@ -6981,6 +6981,17 @@ TR::Register *J9::Power::TreeEvaluator::VMnewEvaluator(TR::Node *node, TR::CodeG
allocateSize = (allocateSize + fej9->getObjectAlignmentInBytes() - 1) & (-fej9->getObjectAlignmentInBytes());
}

if (comp->compileRelocatableCode())
{
switch (opCode)
{
case TR::New: break;
case TR::anewarray: break;
case TR::newarray: break;
default: doInline = false; break;
}
}

static int count = 0;
doInline = doInline && performTransformation(comp, "O^O <%3d> Inlining Allocation of %s [0x%p].\n", count++, node->getOpCode().getName(), node);

Expand Down Expand Up @@ -7032,7 +7043,26 @@ TR::Register *J9::Power::TreeEvaluator::VMnewEvaluator(TR::Node *node, TR::CodeG

insertType = (opCode == TR::newarray && secondChild->getDataType() == TR::Int32 && secondChild->getReferenceCount() == 1 && secondChild->getOpCode().isLoadConst());

if (insertType)
if (comp->compileRelocatableCode() && comp->getOption(TR_UseSymbolValidationManager))
{
// IMPORTANT: secondChild actually references the J9Class of the array *elements* rather
// than the J9Class of the array itself; the new AOT infrastructure requires that
// classReg contain the J9Class of the array, so we have to actually construct a new
// loadaddr for that and then evaluate it instead of evaluating secondChild directly.
// Note that the original secondChild *must still be evaluated* as it will be used in
// the out-of-line code section.
TR::StaticSymbol *classSymbol = TR::StaticSymbol::create(comp->trHeapMemory(), TR::Address);
classSymbol->setStaticAddress(clazz);
classSymbol->setClassObject();

cg->evaluate(secondChild);

This comment has been minimized.

Sign in to view

Copy link
@gita-omr

gita-omr Feb 21, 2019

Contributor

I am a bit unclear: 1) we did not evaluate second child before 2) we evaluate now but don’t use the result

This comment has been minimized.

Sign in to view

Copy link
@aviansie-ben

aviansie-ben Feb 21, 2019

Author Contributor

No, we were always evaluating the second child under these conditions, as its value is needed in the out-of-line section used for the cold path and we cannot evaluate it in an out-of-line code section. The difference now is that classReg no longer holds the value evaluated here. As the comment above says, secondChild is a node containing the address of the component class, but under SVM AOT, classReg is expected to hold the address of the array class.
secondChild = TR::Node::createWithSymRef(TR::loadaddr, 0,
new (comp->trHeapMemory()) TR::SymbolReference(comp->getSymRefTab(), classSymbol));
secondChild->incReferenceCount();

classReg = cg->evaluate(secondChild);
}
else if (insertType)
{
classReg = cg->allocateRegister();
}
Expand Down Expand Up @@ -7113,7 +7143,7 @@ TR::Register *J9::Power::TreeEvaluator::VMnewEvaluator(TR::Node *node, TR::CodeG
// Align the array if necessary.
if (doublewordAlign && !comp->getOptions()->realTimeGC())
genAlignArray(node, iCursor, isVariableLen, resReg, objectSize, dataBegin, dataSizeReg, condReg, tmp5Reg, tmp4Reg, cg);
if (cg->comp()->compileRelocatableCode() && opCode == TR::anewarray)
if (cg->comp()->compileRelocatableCode() && (opCode == TR::anewarray || comp->getOption(TR_UseSymbolValidationManager)))
genInitArrayHeader(node, iCursor, isVariableLen, clazz, classReg, resReg, zeroReg, condReg, enumReg, dataSizeReg, tmp5Reg, tmp4Reg, conditions, needZeroInit, cg);
else
genInitArrayHeader(node, iCursor, isVariableLen, clazz, NULL, resReg, zeroReg, condReg, enumReg, dataSizeReg,
Expand Down Expand Up @@ -7426,11 +7456,14 @@ TR::Register *J9::Power::TreeEvaluator::VMnewEvaluator(TR::Node *node, TR::CodeG
if (cg->comp()->compileRelocatableCode() && (opCode == TR::New || opCode == TR::anewarray))
{
firstInstruction = firstInstruction->getNext();
TR_OpaqueClassBlock *classToValidate = clazz;

TR_RelocationRecordInformation *recordInfo = (TR_RelocationRecordInformation *) comp->trMemory()->allocateMemory(sizeof(TR_RelocationRecordInformation), heapAlloc);
recordInfo->data1 = allocateSize;
recordInfo->data2 = node->getInlinedSiteIndex();
recordInfo->data3 = (uintptr_t) callLabel;
recordInfo->data4 = (uintptr_t) firstInstruction;

TR::SymbolReference * classSymRef;
TR_ExternalRelocationTargetKind reloKind;

Expand All @@ -7443,6 +7476,15 @@ TR::Register *J9::Power::TreeEvaluator::VMnewEvaluator(TR::Node *node, TR::CodeG
{
classSymRef = node->getSecondChild()->getSymbolReference();
reloKind = TR_VerifyRefArrayForAlloc;

if (comp->getOption(TR_UseSymbolValidationManager))
classToValidate = comp->fej9()->getComponentClassFromArrayClass(classToValidate);
}

if (comp->getOption(TR_UseSymbolValidationManager))
{
TR_ASSERT_FATAL(classToValidate, "classToValidate should not be NULL, clazz=%p\n", clazz);
recordInfo->data5 = (uintptr_t)classToValidate;
}

cg->addExternalRelocation(new (cg->trHeapMemory()) TR::BeforeBinaryEncodingExternalRelocation(firstInstruction, (uint8_t *) classSymRef, (uint8_t *) recordInfo, reloKind, cg),
Expand Down Expand Up @@ -7481,6 +7523,8 @@ TR::Register *J9::Power::TreeEvaluator::VMnewEvaluator(TR::Node *node, TR::CodeG
else
{
cg->decReferenceCount(secondChild);
if (node->getSecondChild() != secondChild)
cg->decReferenceCount(node->getSecondChild());
if (classReg != secondChild->getRegister())
cg->stopUsingRegister(classReg);
}
Expand Down

0 comments on commit 715b9d3

Please sign in to comment.