openj9
openj9 copied to clipboard
Verify checkFieldAlignmentForAtomicLong() for inlining atomics
The method checkFieldAlignmentForAtomicLong() checks for a multiple of 4:
bool
J9::Z::CodeGenerator::checkFieldAlignmentForAtomicLong()
{
TR_OpaqueClassBlock * classBlock = self()->comp()->fej9()->getSystemClassFromClassName("java/util/concurrent/atomic/AtomicLong", 38, true);
// TR_J9SharedCacheVM::getSystemClassFromClassName can return 0 when it's impossible to relocate a J9Class later for AOT loads.
if (!classBlock)
return false;
char* fieldName = "value";
int32_t fieldNameLen = 5;
char * fieldSig = "J";
int32_t fieldSigLen = 1;
int32_t intOrBoolOffset = self()->fe()->getObjectHeaderSizeInBytes() + self()->fej9()->getInstanceFieldOffset(classBlock, fieldName, fieldNameLen, fieldSig, fieldSigLen);
return (intOrBoolOffset & 0x3) == 0;
}
However, as this is checking for alignment for an 8 byte field is should probably be checking for a multiple of 8.
Additionally, there is no similar check happening for AtomicInteger in J9::Z::CodeGenerator::InlineDirecCall:
case TR::java_util_concurrent_atomic_AtomicBoolean_getAndSet:
case TR::java_util_concurrent_atomic_AtomicInteger_getAndAdd:
case TR::java_util_concurrent_atomic_AtomicInteger_getAndIncrement:
case TR::java_util_concurrent_atomic_AtomicInteger_getAndDecrement:
case TR::java_util_concurrent_atomic_AtomicInteger_getAndSet:
case TR::java_util_concurrent_atomic_AtomicInteger_addAndGet:
case TR::java_util_concurrent_atomic_AtomicInteger_incrementAndGet:
case TR::java_util_concurrent_atomic_AtomicInteger_decrementAndGet:
resultReg = TR::TreeEvaluator::inlineAtomicOps(node, cg, 4, methodSymbol);
return true;
break;
case TR::java_util_concurrent_atomic_AtomicIntegerArray_getAndAdd:
case TR::java_util_concurrent_atomic_AtomicIntegerArray_getAndIncrement:
case TR::java_util_concurrent_atomic_AtomicIntegerArray_getAndDecrement:
case TR::java_util_concurrent_atomic_AtomicIntegerArray_getAndSet:
case TR::java_util_concurrent_atomic_AtomicIntegerArray_addAndGet:
case TR::java_util_concurrent_atomic_AtomicIntegerArray_incrementAndGet:
case TR::java_util_concurrent_atomic_AtomicIntegerArray_decrementAndGet:
resultReg = TR::TreeEvaluator::inlineAtomicOps(node, cg, 4, methodSymbol, true);
return true;
break;
case TR::java_util_concurrent_atomic_AtomicLong_addAndGet:
case TR::java_util_concurrent_atomic_AtomicLong_getAndAdd:
case TR::java_util_concurrent_atomic_AtomicLong_incrementAndGet:
case TR::java_util_concurrent_atomic_AtomicLong_getAndIncrement:
case TR::java_util_concurrent_atomic_AtomicLong_decrementAndGet:
case TR::java_util_concurrent_atomic_AtomicLong_getAndDecrement:
if (cg->checkFieldAlignmentForAtomicLong() && comp->target().cpu.isAtLeast(OMR_PROCESSOR_S390_Z196))
{
// TODO: I'm not sure we need the z196 restriction here given that the function already checks for z196 and
// has a compare and swap fallback path
resultReg = TR::TreeEvaluator::inlineAtomicOps(node, cg, 8, methodSymbol);
return true;
}
break;
offset being verified also seems strange: objectheadersize + fieldoffset (instead of just field?)
Opening this issue to track investigation of the correct offset and if the verification is needed for AtomicInteger
@r30shah @keithc-ca