Skip to content

Commit

Permalink
Guarantee functional correctness of rdbar/wrtbar
Browse files Browse the repository at this point in the history
This change guarantee the correctness of rdbar/wrtbar from the following
aspects:
1. Consider rdbar/wrtbar as GC points
2. Make optimizations consider shape of rdbar/wrtbar when doing
transformations
3. Disable opts not supporting newly added rdbar/wrtbar when seeing
the opcodes

Signed-off-by: Yi Zhang <yizhang@ca.ibm.com>
  • Loading branch information
Yi Zhang committed Jan 4, 2019
1 parent bbe9e12 commit 5d57737
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 11 deletions.
22 changes: 14 additions & 8 deletions runtime/compiler/codegen/J9CodeGenerator.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2018 IBM Corp. and others
* Copyright (c) 2000, 2019 IBM Corp. and others
*
* This program and the accompanying materials are made available under
* the terms of the Eclipse Public License 2.0 which accompanies this
Expand Down Expand Up @@ -351,7 +351,7 @@ J9::CodeGenerator::lowerCompressedRefs(

// base object
address = loadOrStoreNode->getFirstChild();
loadOrStoreOp = TR::Compiler->om.shouldGenerateReadBarriersForFieldLoads() ? self()->comp()->il.opCodeForIndirectReadBarrier(TR::Int32) :
loadOrStoreOp = TR::Compiler->om.shouldGenerateReadBarriersForFieldLoads() || loadOrStoreNode->getOpCode().isReadBar() ? self()->comp()->il.opCodeForIndirectReadBarrier(TR::Int32) :
self()->comp()->il.opCodeForIndirectLoad(TR::Int32);
}
else if ((loadOrStoreNode->getOpCode().isStoreIndirect() ||
Expand Down Expand Up @@ -2926,13 +2926,19 @@ J9::CodeGenerator::compressedReferenceRematerialization()

static bool disableRematforCP = feGetEnv("TR_DisableWrtBarOpt") != NULL;

if (TR::Compiler->om.shouldGenerateReadBarriersForFieldLoads())
// The compressedrefs remat opt removes decompression/compression sequences from
// loads/stores where there doesn't exist a gc point between the load and the store,
// and the load doesn't need to be dereferenced.
// The opt needs to be disabled for the following cases:
// 1. In Guarded Storage, we can't not do a guarded load because the object that is loaded may
// not be in the root set, and as a consequence, may get moved.
// 2. For read barriers in field watch, the vmhelpers are GC points and therefore the object might be moved
if (TR::Compiler->om.shouldGenerateReadBarriersForFieldLoads() || self()->comp()->getOption(TR_EnableFieldWatch))
{
// We need this restriction because the compressedrefs remat opt
// removes decompression/compression sequences from loads/stores where there doesn't exist
// a gc point between the load and the store, and the load doesn't need to be dereferenced.
// In Guarded Storage, we can't not do a guarded load because the object that is loaded may
// not be in the root set, and as a consequence, may get moved.
if (TR::Compiler->om.shouldGenerateReadBarriersForFieldLoads())
traceMsg(self()->comp(), "The compressedrefs remat opt is disabled because Concurrent Scavenger is enabled\n");
if (self()->comp()->getOption(TR_EnableFieldWatch))
traceMsg(self()->comp(), "The compressedrefs remat opt is disabled because field watch is enabled\n");
disableRematforCP = true;
}

Expand Down
6 changes: 6 additions & 0 deletions runtime/compiler/compile/J9Compilation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1398,3 +1398,9 @@ J9::Compilation::notYetRunMeansCold()
return true;
}

bool
J9::Compilation::incompleteOptimizerSupportForReadWriteBarriers()
{
return self()->getOption(TR_EnableFieldWatch);
}

2 changes: 2 additions & 0 deletions runtime/compiler/compile/J9Compilation.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,8 @@ class OMR_EXTENSIBLE Compilation : public OMR::CompilationConnector

TR_RelocationRuntime *reloRuntime() { return _reloRuntime; }

bool incompleteOptimizerSupportForReadWriteBarriers();

TR::SymbolValidationManager *getSymbolValidationManager() { return _symbolValidationManager; }

private:
Expand Down
8 changes: 7 additions & 1 deletion runtime/compiler/optimizer/EscapeAnalysis.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2018 IBM Corp. and others
* Copyright (c) 2000, 2019 IBM Corp. and others
*
* This program and the accompanying materials are made available under
* the terms of the Eclipse Public License 2.0 which accompanies this
Expand Down Expand Up @@ -179,6 +179,12 @@ int32_t TR_EscapeAnalysis::perform()
if (comp()->isOptServer() && (comp()->getMethodHotness() <= warm))
return 0;

// EA generates direct stores/loads to instance field which is different
// from a normal instance field read/write. Field watch would need special handling
// for stores/loads genearted by EA.
if (comp()->incompleteOptimizerSupportForReadWriteBarriers())
return 0;

static char *doESCNonQuiet = feGetEnv("TR_ESCAPENONQUIET");
if (doESCNonQuiet && comp()->getOutFile() == NULL)
return 0;
Expand Down
6 changes: 5 additions & 1 deletion runtime/compiler/optimizer/IdiomRecognition.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2018 IBM Corp. and others
* Copyright (c) 2000, 2019 IBM Corp. and others
*
* This program and the accompanying materials are made available under
* the terms of the Eclipse Public License 2.0 which accompanies this
Expand Down Expand Up @@ -2302,6 +2302,10 @@ TR_CISCNode *
TR_CISCTransformer::addAllSubNodes(TR_CISCGraph *const graph, TR::Block *const block, TR::TreeTop *const top,
TR::Node *const parent, TR::Node *const node, const int32_t dagId)
{
//IdiomRecognition doesn't know how to handle rdbar/wrtbar for now
if (comp()->incompleteOptimizerSupportForReadWriteBarriers() &&
(node->getOpCode().isReadBar() || node->getOpCode().isWrtBar()))
return 0;
int32_t i;
int32_t numChildren = node->getNumChildren();
TR_ScratchList<TR_CISCNode> childList(trMemory());
Expand Down
14 changes: 13 additions & 1 deletion runtime/compiler/optimizer/J9TransformUtil.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2018 IBM Corp. and others
* Copyright (c) 2000, 2019 IBM Corp. and others
*
* This program and the accompanying materials are made available under
* the terms of the Eclipse Public License 2.0 which accompanies this
Expand Down Expand Up @@ -1077,6 +1077,16 @@ J9::TransformUtil::canFoldStaticFinalField(TR::Compilation *comp, TR::Node* node
return TR_maybe;
}

/*
* Load const node should have zero children
*/
static void prepareNodeToBeLoadConst(TR::Node *node)
{
for (int i=0; i < node->getNumChildren(); i++)
node->getAndDecChild(i);
node->setNumChildren(0);
}

bool
J9::TransformUtil::foldStaticFinalFieldImpl(TR::Compilation *comp, TR::Node *node)
{
Expand Down Expand Up @@ -1143,6 +1153,7 @@ J9::TransformUtil::foldStaticFinalFieldImpl(TR::Compilation *comp, TR::Node *nod
if (performTransformation(comp, "O^O foldStaticFinalField: turn [%p] %s %s into load const\n", node, node->getOpCode().getName(), symRef->getName(comp->getDebug())))
{
TR::VMAccessCriticalSection isConsitble(comp->fej9());
prepareNodeToBeLoadConst(node);
switch (loadType)
{
case TR::Int8:
Expand Down Expand Up @@ -1191,6 +1202,7 @@ J9::TransformUtil::foldStaticFinalFieldImpl(TR::Compilation *comp, TR::Node *nod
default:
if (performTransformation(comp, "O^O transformDirectLoad: [%p] field is null - change to aconst NULL\n", node))
{
prepareNodeToBeLoadConst(node);
TR::Node::recreate(node, TR::aconst);
node->setAddress(0);
node->setIsNull(true);
Expand Down

0 comments on commit 5d57737

Please sign in to comment.