Handle 1D arrays with multianewarray #17358
Conversation
|
Personal build 1512 with sanity.functional ran successfully on x86-64 linux |
JamesKingdon
changed the title
| * | ||
| * NB Must only be used for arrays of at least two dimensions | ||
| */ | ||
| static TR::Register * generateMultianewArrayWithInlineAllocators(TR::Compilation *comp, TR::Node *node, TR::CodeGenerator *cg) |
There was a problem hiding this comment.
You shouldn't pass both a Compilation and a CodeGenerator object. In the code generator, you typically pass the CodeGenerator object and can get the Compilation object via cg->comp().
| TR::Node *thirdChild = node->getThirdChild(); | ||
| TR::Node *firstChild = node->getFirstChild(); // ptr to array of sizes, one for each dimension. Array construction stops at the outermost zero size | ||
| TR::Node *secondChild = node->getSecondChild(); // Number of dimensions - this is fixed in the bytecode, so compile time constant | ||
| TR::Node *thirdChild = node->getThirdChild(); // class of the outer most dimension |
| TR::Node *secondChild = node->getSecondChild(); // Number of dimensions - this is fixed in the bytecode, so compile time constant | ||
|
|
||
| // if secondChild is an iconst we can read the number of dimensions. Only generate inline code if nDims > 1 | ||
| if (secondChild->getOpCodeValue() == TR::iconst) |
There was a problem hiding this comment.
This can't be anything other than a constant. You should make this an assert instead and get rid of the else path.
| } | ||
| else | ||
| { | ||
| // This is directly copied from the evaluator on P and should result in the simplest code to call the helper |
The tree evaluator for multianewarray on X and Z has an optimisation for 0 length arrays that generate inline code for allocating the array. This code assumes that the new array will always have at least two dimensions, however the VM spec permits multianewarray to be used for 1 dimensional arrays. This does not appear to be used by javac, but can appear in code generated by other languages, e.g. Groovy. This commit updates the evaluator on X to check the number of dimensions. For arrays of dimension 2 or more the original code is used with inline allocation where possible, and for 1 dimensional arrays the much simpler code from P is used which simply calls the helper.
JamesKingdon
changed the title
JamesKingdon
force-pushed
the
multianewarrayFix
branch
from
c0f9a35
to
80d9415
Compare
|
|
||
| if (useInlineAllocator) | ||
| { | ||
| return generateMultianewArrayWithInlineAllocators(node, cg); |
There was a problem hiding this comment.
Why do you need useInlineAllocator at all? Just move this return up to where you set it.
There was a problem hiding this comment.
It made a little more sense before your last set of requested changes which I implemented too literally. I'll clean it up
| } | ||
| else | ||
| { | ||
| TR::ILOpCodes opCode = node->getOpCodeValue(); |
There was a problem hiding this comment.
Move this else body up to the else at L1823.
JamesKingdon
force-pushed
the
multianewarrayFix
branch
from
80d9415
to
7014e54
Compare
|
Jenkins test sanity.functional,sanity.system,sanity.openjdk xlinux,win,xmac jdk17 |
|
Jenkins test sanity.openjdk win jdk17 |
|
I did a personal build+sanity on x86-64 linux for Java8 and 11. Java 8 passed, Java11 failed some tests for criu. build # 16972 |
The tree evaluator for multianewarray on X and Z has an optimisation for 0 length arrays that generate inline code for allocating the array. This code assumes that the new array will always have at least two dimensions, however the VM spec permits multianewarray to be used for 1 dimensional arrays. This does not appear to be used by javac, but can appear in code generated by other languages, e.g. Groovy.
This commit updates the evaluator on X to check the number of dimensions. For arrays of dimension 2 or more the original code is used with inline allocation where possible, and for 1 dimensional arrays the much simpler code from P is used which simply calls the helper.