Allow some Unsafe atomic ops to be inlined on POWER #3086
Conversation
Allow for the possibility that the parent OMR class will want to inline a direct call by delegating to the parent class's implementation of inlineDirectCall. In particular, we will want to do that for some atomic operations in Unsafe. Signed-off-by: Henry Zongaro <[email protected]>
fjeremic
added
comp:jit
depends:omr
| || TR::Compiler->target.cpu.isPower()) | ||
| && !comp()->getOption(TR_DisableUnsafe) | ||
| && !comp()->compileRelocatableCode() | ||
| && !TR::Compiler->om.canGenerateArraylets(); |
There was a problem hiding this comment.
These queries will need updating after eclipse-omr/omr#2999 is merged. I suggest we mark this PR as WIP until this is done.
|
@hzongaro eclipse-omr/omr#2999 has been merged - can you update to account for that change so that review can progress? |
I think a large part of this PR is made redundant by #3195. @hzongaro given #3195 I think you only need the runtime/compiler/p/codegen/J9TreeEvaluator.cpp changes here. |
Allow for the possibility that the parent OMR class will want to inline a direct call by delegating to the parent class's implementation of inlineDirectCall. In particular, we will want to do that for some atomic operations in Unsafe. Signed-off-by: Henry Zongaro <[email protected]>
…into atomic-power-alt
Removed checks of platform for determining whether atomic helpers are supported. Signed-off-by: Henry Zongaro <[email protected]>
Adjusted call to overridden inlineDirectCall to be done through the connector, rather than calling the implementation of OMR::Power::CodeGenerator::inlineDirectCall directly. Signed-off-by: Henry Zongaro <[email protected]>
hzongaro
changed the title
hzongaro
changed the title
|
I believe all review comments have been addressed, so I will remove "WIP" from the title. |
|
Jenkins test sanity plinux,aix JDK8 |
|
@fjeremic were you going to merge this or do we need to get another opinion before you are ready to merge it? |
|
I would like to take a look before merge. |
|
Just wanted to mention that I merged this PR to fix the build. However, we still need to discuss why this inlineDriectCall check is needed. E.g. it would be good to have only one place where we ask if any particular CodeGenerator can inline these atomic ops. |
|
To follow up on our discussion @hzongaro @fjeremic @0dvictor adding an ASSERT in the evaluator (inlineDirectCall || !supportsNonHelper) would be slightly misleading since supportsNonHelper (at least from its name) does not imply that the helper has to be inlined. Also, I think checking for 64bit should be done inside supports/inlinesNonHelper(). |
Allow for the possibility that the parent OMR class will want to
inline a direct call by delegating to the parent class's implementation
of inlineDirectCall. In particular, we will want to do that for some
atomic operations in Unsafe.
This depends on eclipse-omr/omr#3018
Signed-off-by: Henry Zongaro [email protected]