Consolidate math call transformations in recognizedCallTransformer #2320
Conversation
|
Depends on eclipse-omr/omr#2711 and eclipse-omr/omr#2722 |
fjeremic
added
comp:jit
depends:omr
dhong44
changed the title
| TR::Compiler->target.cpu.isPower() || | ||
| TR::Compiler->target.cpu.isZ() || | ||
| TR::Compiler->target.cpu.isARM() && !disableARMMaxMin) && | ||
| !comp()->getOption(TR_DisableMaxMinOptimization); |
There was a problem hiding this comment.
Are we able to eliminate these platform checks and the disableARMMaxMin static variables by simply setting TR_DisableMaxMinOptimization on ARM? Similarly below.
|
The proposed change implicitly enabled transforming [i/l]abs on X86 while not deleting old related X86 CodeGen logic, it might be better to wait for #2360 |
|
Thank you for waiting. #2360 has now been merged. This PR may need a rebase to reflect the latest change. |
| @@ -48,22 +48,34 @@ void J9::RecognizedCallTransformer::processIntrinsicFunction(TR::TreeTop* treeto | |||
| TR::TransformUtil::removeTree(comp(), treetop); | |||
| } | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
Extra new line not needed.
| TR::Compiler->target.cpu.isPower() || | ||
| TR::Compiler->target.cpu.isZ() || | ||
| TR::Compiler->target.cpu.isARM()) && | ||
| !comp()->getOption(TR_DisableMaxMinOptimization); |
There was a problem hiding this comment.
We still need to fold all of these platform checks.
| case TR::java_lang_Math_max_D: | ||
| case TR::java_lang_Math_min_D: | ||
| return (TR::Compiler->target.cpu.isARM()) && | ||
| !comp()->getOption(TR_DisableMaxMinOptimization); |
There was a problem hiding this comment.
This is already set on ARM so we don't need the isARM platform check.
There was a problem hiding this comment.
Since only arm supports transformation of Max and Min calls, without the isARM check this would always return false. I intended to leave this code so in the future it could be enabled on ARM. Instead, should the cases for floating point max and min calls just be removed?
There was a problem hiding this comment.
Instead, should the cases for floating point max and min calls just be removed?
I think so yes.
Move transformations from java/lang/math calls to opcodes into recognized call transformer optimization and add transformations for Z. Closes: eclipse-openj9#2045 Signed-off-by: Daniel Hong <daniel.hong@live.com>
Remove booleans that were previously used as placeholders for ARM. Setting options to disable features will be done with OMR Options instead. Signed-off-by: Daniel Hong <daniel.hong@live.com>
Remove platform check in J9RecognizedCallTransformer for max and min operations, since all platforms support call transformation. Signed-off-by: Daniel Hong <daniel.hong@live.com>
Remove cases in recognized call transformer dealing with max/min calls using floating point numbers. Signed-off-by: Daniel Hong <daniel.hong@live.com>
|
Jenkins test sanity |
|
Windows failures due to #2129. |
Move transformations from java/lang/math calls to opcodes
into recognized call transformer optimization and add
transformations for Z.
Closes: #2045
Signed-off-by: Daniel Hong daniel.hong@live.com