Fix EA bug in generating VSPLATS #15709
Conversation
There was a problem hiding this comment.
I think the change looks good, but I noticed another location in EscapeAnalysis that uses vectorToScalar rather than getVectorElementType. Is vectorToScalar deprecated? If so, maybe you could update this change to take the opportunity to replace it with a call to getVectorElementType as well.
|
@hzongaro looking at the vectorToScalar , it eventually returns |
I think |
| @@ -6242,7 +6242,7 @@ TR::Node *TR_EscapeAnalysis::createConst(TR::Compilation *comp, TR::Node *node, | |||
| if (type.isVector()) | |||
| { | |||
| result = TR::Node::create(node, TR::ILOpCode::createVectorOpCode(TR::vsplats, type), 1); | |||
| result->setAndIncChild(0, TR::Node::create(node, comp->il.opCodeForConst(type), value)); | |||
| result->setAndIncChild(0, TR::Node::create(node, comp->il.opCodeForConst(type.getVectorElementType()), value)); | |||
There was a problem hiding this comment.
LGTM. Just curious: which test case caused this code to be executed?
There was a problem hiding this comment.
I was seeing the assert being thrown while compiling this method https://github.com/ibmruntimes/openj9-openjdk-jdk18/blob/9bdb9425c548ea2917c5b79ecbb5788b86314c00/test/jdk/jdk/incubator/vector/Int128VectorTests.java#L5089-L5098 in BumbleBench
In escape analyis while generating constant child of VSPLATS node, it queries the IL API to get the opcode using the Vector IL instead of Vector Element type. This resulted in us hitting fatal assert. This commit fixes by supplying the element type to getOpCode query for constant node. Apart from fixing the bug, this commit replaces the use of vectorToScalar query with getVectorElementType in escape analysis optimization. Signed-off-by: Rahil Shah <rahil@ca.ibm.com>
r30shah
force-pushed
the
fixEscapeAnalysisVector
branch
from
821e45f
to
9dee936
Compare
| @@ -3871,7 +3871,7 @@ void TR_EscapeAnalysis::referencedField(TR::Node *base, TR::Node *field, bool is | |||
| int N = 1; | |||
| if (refType.isVector()) | |||
| { | |||
| fieldType = refType.vectorToScalar(); | |||
| fieldType = refType.getVectorElementType(); | |||
There was a problem hiding this comment.
@hzongaro given what Gita suggested, I think it makes sense to replace vectorToScalar query with getVectorElementType. I fixed the second occurrence that you pointed in review in https://github.com/eclipse-openj9/openj9/compare/821e45f1c7c9de5e6636985a4b59cdf2fb0979a6..9dee936f8cf7248144b47d73c71d919dfa443a97
|
Gita @gita-omr, I just wanted to close the loop, and make sure you were OK with the latest update to this pull request. |
|
LGTM |
|
Jenkins test sanity all jdk8,jdk18 |
In escape analyis while generating constant child of VSPLATS node, it
queries the IL API to get the opcode using the Vector IL instead of
Vector Element type. This resulted in us hitting fatal assert. This
commit fixes by supplying the element type to getOpCode query for
constant node.
Signed-off-by: Rahil Shah rahil@ca.ibm.com