pshipton
commented
2 years ago @0xdaryl
Closed Spencer-Comin closed 2 years ago
pshipton
commented
2 years ago @0xdaryl
r30shah
commented
2 years ago @Spencer-Comin Do you have a jitdump from the failure? Which we can inspect? Code in which assert is thrown is only used by Z and Power to generate list of tests for type testing (Instanceof/checkCast).
Spencer-Comin
commented
2 years ago I just tried another debug build with most recent OMR and OpenJ9 and it built successfully. However, looking at the original failure message, I came up with a unit test to trigger similar compilation of the failing method (MethodHandles.insertArguments()):
import java.lang.invoke.*;
class InsertArgumentsTest {
static int counter = 0;
static class Dummy {
static void increment(int i) {
counter += i;
}
}
public static void main(String[] args) throws IllegalAccessException, NoSuchMethodException, Throwable {
MethodHandle master = MethodHandles.lookup().findStatic(
Dummy.class,
"increment",
MethodType.methodType(void.class, int.class)
);
MethodHandle mh;
for (int i = 0; i < 100000; i++) {
mh = MethodHandles.insertArguments(master, 0, i);
mh.invoke();
}
System.out.println(counter);
}
}This unit test still fails with the debug java when run with java -Xjit:"disableAsyncCompilation,dontInline={*methodHandles.insertArguments*},{*MethodHandles.insertArguments*}(disableInlining,traceFull,log=reference.log),verbose,vlog=vlog" InsertArgumentsTest
Here's the end of the jitdump:
------------------------------
n347n ( 0) checkcast [#86] [ 0x3feff075c30] bci=[-1,155,3166] rc=0 vc=891 vn=- li=19 udi=- nc=2
n632n ( 1) ==>aRegLoad (in &GPR_1187) (SeenRealReference sharedMemory )
n346n ( 1) aloadi <classFromJavaLangClass>[#301 Shadow +8] [flags 0x10607 0x0 ] (X!=0 sharedMemory ) [ 0x3feff075be0] bci=[-1,151,3166] rc=1 vc=891 vn=- li=19 udi=- nc=1 flg=0x4
n631n ( 1) ==>aRegLoad (in &GPR_1186) (X!=0 X>=0 SeenRealReference sharedMemory )
------------------------------
checkcast:Maximum Profiled Classes = 3
Cast Class runtimeVariable
Assertion failed at /jit/team/spencer/11/reference/openj9/runtime/compiler/codegen/J9TreeEvaluator.cpp:1343:isInstanceOf:
Expecting instanceof when cast class is a runtime variableFull log and dump files: vlog.txt reference.log.txt javacore.20220705.085412.38779.0002.txt jitdump.20220705.085412.38779.0004.dmp.txt
r30shah
commented
2 years ago @Spencer-Comin Thanks a lot for the unit test, I believe the change that lowers the Class.cast calls to checkCast (https://github.com/eclipse-openj9/openj9/commit/f68fcd5d63840563c7f5a93607049308078f391e) introduced a checkCast node where the Cast Class is runtime variable. Looking at the code that contains the ASSERT [1], I think it should be OK to remove that soft ASSERT. Only thing that requires some investigations is the need to generate SuperClassTest [2] which on Z we generate even for Dynamic Cast Class and the test at runtime checks if it is normal class or array class before checking out the super class array. Also currently checkCast evaluator on Z do not generate instructions for DynamicCache for Dynamic Cast class for CheckCast, which would be in the list of suggested tests to be generated [3], so we probably should check if we can extend use of dynamic cache to checkCast and there is a benefit from doing so.
@Spencer-Comin as you have unit test handy, can I request you to checkout what kind of sequence we generate when Cast.class is reduced to checkCast and verify functionality on Z?
[1]. https://github.com/eclipse-openj9/openj9/blob/a9bdd6d1aee01a07acc975d410ce57a85bbde571/runtime/compiler/codegen/J9TreeEvaluator.cpp#L1343 [2]. https://github.com/eclipse-openj9/openj9/blob/a9bdd6d1aee01a07acc975d410ce57a85bbde571/runtime/compiler/codegen/J9TreeEvaluator.cpp#L1349-L1352 [3]. https://github.com/eclipse-openj9/openj9/blob/a9bdd6d1aee01a07acc975d410ce57a85bbde571/runtime/compiler/codegen/J9TreeEvaluator.cpp#L1353-L1354
Spencer-Comin
commented
2 years ago Here's the instruction generation with the assert removed:
------------------------------
n369n ( 0) checkcast [#86] [ 0x3ff5e175310] bci=[-1,155,3166] rc=0 vc=856 vn=- li=19 udi=- nc=2
n539n ( 1) ==>aRegLoad (in &GPR_0371) (SeenRealReference sharedMemory )
n192n ( 1) ==>aloadi (in GPR_0375) (X!=0 X>=0 sharedMemory )
------------------------------
checkcast:Maximum Profiled Classes = 3
Cast Class runtimeVariable
Outline Super Class Test: 0
checkcast: Class Not Evaluated. Evaluating it
checkcast: Emitting NullTest
checkcast: Loading Object Class
checkcast: Emitting Class Equality Test
checkcast: Emitting CastClassCacheTest
checkcast: Emitting helper call
checkcast: Internal Control Flow in OOL : false
ReturnType = Address
------------------------------
n369n ( 0) checkcast [#86] [ 0x3ff5e175310] bci=[-1,155,3166] rc=0 vc=856 vn=- li=19 udi=- nc=2
n539n ( 0) ==>aRegLoad (in &GPR_0371) (SeenRealReference sharedMemory )
n192n ( 0) ==>aloadi (in GPR_0375) (X!=0 X>=0 sharedMemory )
------------------------------
[ 0x3ff5e4665d0] LTGR &GPR_0371,&GPR_0371
[ 0x3ff5e4666b0] BRC BH(0x8), Label L0080
[ 0x3ff5e4668c0] LLZRGF GPR_0384,#471 0(&GPR_0371)
[ 0x3ff5e466990] CGRJ GPR_0375,GPR_0384,Label L0080,BH(mask=0x8),
[ 0x3ff5e466bd0] LG GPR_0385,#472 200(GPR_0384)
[ 0x3ff5e466ca0] CGRJ GPR_0385,GPR_0375,Label L0080,BH(mask=0x8),
[ 0x3ff5e466f70] BRC NOP(0xf), Outlined Label L0081
[ 0x3ff5e46a880] assocreg
[ 0x3ff5e46a2c0] Label L0080:
POST:
{AssignAny:&GPR_0371:R} {AssignAny:GPR_0384:R} {AssignAny:GPR_0385:R} {AssignAny:GPR_0375:R}
------------------------------The unit test runs as expected without failures
Full logs: reference.log.txt vlog.txt
r30shah
commented
2 years ago @Spencer-Comin Thanks a lot for trying this out quickly. I suggest you can remove the ASSERT from that code and create a PR referencing https://github.com/eclipse-openj9/openj9/commit/f68fcd5d63840563c7f5a93607049308078f391e.
Even though the transformation is better than call to Class.cast, we should be able to improve the performance of such checkcast node further (Some of the easy ones are noted in https://github.com/eclipse-openj9/openj9/issues/15449#issuecomment-1175354884). Can you open up an issue for that?
Internal JDK11 debug build on z/Linux failed on assert at
runtime/compiler/codegen/J9TreeEvaluator.cpp:1343: isInstanceOfSteps to reproduce:
export cflags="-Og -ggdb3 -fno-inline -DDEBUG"export EXTRA_CMAKE_ARGS="-DJ9JIT_EXTRA_CFLAGS=\"$cflags\" -DJ9JIT_EXTRA_CXXFLAGS=\"$cflags\" -DOMR_SEPARATE_DEBUG_INFO=OFF"