[PATCH] 8230797: ARM32-softfp: assertion in InterpreterRuntime::resolve_ldc
christoph.goettschkes at microdoc.com
christoph.goettschkes at microdoc.com
Fri Sep 13 07:54:05 UTC 2019
Hi Boris,
I investigated all failing tests now and fixed 4 more issues. Now the HotSpot
tier1 JTreg tests are passing (except some tests because of memory
restrictions and the like).
The changes are in different places. Two are in the C1 JIT, one in shared code
of the runtime and one in a test for the JIT compiler.
I created 4 more patches, one for every problem I found. Should I ask for new
bugs in the JBS for these problems, or could you just create 4 webrevs and I
RFR using the already existing bug?
#1 Wrong assumption in assertion in LIRGenerator::atomic_xchg() [1] and LIRGenerator::atomic_add() [2]
The assertion assumes that atomic operations on 32-bit ARM are only possible
for 32-bit types. This is only true, if the instructions LDREXD/STREXB are
not present on the target platform. I added a check which uses
VM_Version::supports_ldrexd() to see if those instructions are available.
Other parts of the C1 JIT already make use of this check.
This patch fixes the following tests:
compiler/codegen/Test8011901.java
compiler/unsafe/JdkInternalMiscUnsafeAccessTestDouble.java
runtime/exceptionMsgs/ArrayIndexOutOfBoundsException/ArrayIndexOutOfBoundsExceptionTest.java#id1
diff --git a/src/hotspot/cpu/arm/c1_LIRGenerator_arm.cpp b/src/hotspot/cpu/arm/c1_LIRGenerator_arm.cpp
--- a/src/hotspot/cpu/arm/c1_LIRGenerator_arm.cpp
+++ b/src/hotspot/cpu/arm/c1_LIRGenerator_arm.cpp
@@ -774,7 +774,7 @@
bool is_oop = type == T_OBJECT || type == T_ARRAY;
LIR_Opr result = new_register(type);
value.load_item();
- assert(type == T_INT || is_oop LP64_ONLY( || type == T_LONG ), "unexpected type");
+ assert(type == T_INT || is_oop || (type == T_LONG && VM_Version::supports_ldrexd()), "unexpected type");
LIR_Opr tmp = (UseCompressedOops && is_oop) ? new_pointer_register() : LIR_OprFact::illegalOpr;
__ xchg(addr, value.result(), result, tmp);
return result;
@@ -783,7 +783,7 @@
LIR_Opr LIRGenerator::atomic_add(BasicType type, LIR_Opr addr, LIRItem& value) {
LIR_Opr result = new_register(type);
value.load_item();
- assert(type == T_INT LP64_ONLY( || type == T_LONG), "unexpected type");
+ assert(type == T_INT || (type == T_LONG && VM_Version::supports_ldrexd()), "unexpected type");
LIR_Opr tmp = new_register(type);
__ xadd(addr, value.result(), result, tmp);
return result;
#2 Wrong assumption in assertion in oop::register_oop() [3]
oop::register_oop() is only used in fastdebug builds for the CheckUnhandledOops
feature. The assert requires the PC of the current frame to be non-zero. In
32-bit ARM, this is only true if the VM is not compiled in thumb mode. In thumb
mode, os::current_frame() [4] always returns a default frame, in which the PC
(and every other field) is zero.
Since the PC is only used for reporting, and the assumption in the assert is
not true on all supported platforms, I removed the assert altogether.
This patch fixes the following tests:
runtime/CheckUnhandledOops/TestOutOfMemory.java
diff --git a/src/hotspot/share/oops/oopsHierarchy.cpp b/src/hotspot/share/oops/oopsHierarchy.cpp
--- a/src/hotspot/share/oops/oopsHierarchy.cpp
+++ b/src/hotspot/share/oops/oopsHierarchy.cpp
@@ -40,8 +40,6 @@
Thread* t = Thread::current_or_null();
if (t != NULL && t->is_Java_thread()) {
frame fr = os::current_frame();
- // This points to the oop creator, I guess current frame points to caller
- assert (fr.pc(), "should point to a vm frame");
t->unhandled_oops()->register_unhandled_oop(this, fr.pc());
}
}
#3 Test compiler/codegen/TestCharVect2.java only works with server VMs.
The test uses the HotSpot option MaxVectorSize, which is only available in
server VMs. I added the flag IgnoreUnrecognizedVMOptions to make the test
pass in client VMs.
This patch fixes the following tests:
compiler/codegen/TestCharVect2.java
diff --git a/test/hotspot/jtreg/compiler/codegen/TestCharVect2.java b/test/hotspot/jtreg/compiler/codegen/TestCharVect2.java
--- a/test/hotspot/jtreg/compiler/codegen/TestCharVect2.java
+++ b/test/hotspot/jtreg/compiler/codegen/TestCharVect2.java
@@ -24,12 +24,12 @@
/**
* @test
* @bug 8001183
- * @summary incorrect results of char vectors right shift operaiton
+ * @summary incorrect results of char vectors right shift operation
*
* @run main/othervm/timeout=400 -Xbatch -Xmx128m compiler.codegen.TestCharVect2
- * @run main/othervm/timeout=400 -Xbatch -Xmx128m -XX:MaxVectorSize=8 compiler.codegen.TestCharVect2
- * @run main/othervm/timeout=400 -Xbatch -Xmx128m -XX:MaxVectorSize=16 compiler.codegen.TestCharVect2
- * @run main/othervm/timeout=400 -Xbatch -Xmx128m -XX:MaxVectorSize=32 compiler.codegen.TestCharVect2
+ * @run main/othervm/timeout=400 -Xbatch -Xmx128m -XX:+IgnoreUnrecognizedVMOptions -XX:MaxVectorSize=8 compiler.codegen.TestCharVect2
+ * @run main/othervm/timeout=400 -Xbatch -Xmx128m -XX:+IgnoreUnrecognizedVMOptions -XX:MaxVectorSize=16 compiler.codegen.TestCharVect2
+ * @run main/othervm/timeout=400 -Xbatch -Xmx128m -XX:+IgnoreUnrecognizedVMOptions -XX:MaxVectorSize=32 compiler.codegen.TestCharVect2
*/
package compiler.codegen;
#4 Address displacement is 0 for volatile field access because of Unsafe field access.
In the LIRGenerator::volatile_field_store() [5] and
LIRGenerator::volatile_field_load() [6], the target address is calculated by
using the function add_large_constant(), which asserts that the given
displacement is not 0. But in the implementation of the intrinsic for the
Unsafe class, this can actually happen. I added a check in
LIRGenerator::volatile_field_store() and LIRGenerator::volatile_field_load()
which makes the implementation only call add_large_constant() if the
displacement is non zero. If the displacement is 0, no calculation is required.
This patch fixes the following tests:
compiler/codegen/CRCTest.java
compiler/intrinsics/bigInteger/TestMulAdd.java
serviceability/jvmti/RedefineClasses/TestMultipleClasses.java
compiler/unsafe/JdkInternalMiscUnsafeAccessTestLong.java
compiler/unsafe/SunMiscUnsafeAccessTestLong.java
gc/epsilon/TestByteArrays.java
gc/epsilon/TestObjects.java
gc/epsilon/TestRefArrays.java
diff --git a/src/hotspot/cpu/arm/c1_LIRGenerator_arm.cpp b/src/hotspot/cpu/arm/c1_LIRGenerator_arm.cpp
--- a/src/hotspot/cpu/arm/c1_LIRGenerator_arm.cpp
+++ b/src/hotspot/cpu/arm/c1_LIRGenerator_arm.cpp
@@ -1310,9 +1310,16 @@
CodeEmitInfo* info) {
if (value->is_double_cpu()) {
assert(address->index()->is_illegal(), "should have a constant displacement");
- LIR_Opr tmp = new_pointer_register();
- add_large_constant(address->base(), address->disp(), tmp);
- __ volatile_store_mem_reg(value, new LIR_Address(tmp, (intx)0, address->type()), info);
+ LIR_Address* store_addr = NULL;
+ if (address->disp() != 0) {
+ LIR_Opr tmp = new_pointer_register();
+ add_large_constant(address->base(), address->disp(), tmp);
+ store_addr = new LIR_Address(tmp, (intx)0, address->type());
+ } else {
+ // address->disp() can be 0, if the address is referenced using the unsafe intrinsic
+ store_addr = address;
+ }
+ __ volatile_store_mem_reg(value, store_addr, info);
return;
}
__ store(value, address, info, lir_patch_none);
@@ -1322,9 +1329,16 @@
CodeEmitInfo* info) {
if (result->is_double_cpu()) {
assert(address->index()->is_illegal(), "should have a constant displacement");
- LIR_Opr tmp = new_pointer_register();
- add_large_constant(address->base(), address->disp(), tmp);
- __ volatile_load_mem_reg(new LIR_Address(tmp, (intx)0, address->type()), result, info);
+ LIR_Address* load_addr = NULL;
+ if (address->disp() != 0) {
+ LIR_Opr tmp = new_pointer_register();
+ add_large_constant(address->base(), address->disp(), tmp);
+ load_addr = new LIR_Address(tmp, (intx)0, address->type());
+ } else {
+ // address->disp() can be 0, if the address is referenced using the unsafe intrinsic
+ load_addr = address;
+ }
+ __ volatile_load_mem_reg(load_addr, result, info);
return;
}
__ load(address, result, info, lir_patch_none);
Best regards,
Christoph
[1] http://hg.openjdk.java.net/jdk/jdk/file/9b4717ca9bd1/src/hotspot/cpu/arm/c1_LIRGenerator_arm.cpp#l773
[2] http://hg.openjdk.java.net/jdk/jdk/file/9b4717ca9bd1/src/hotspot/cpu/arm/c1_LIRGenerator_arm.cpp#l783
[3] http://hg.openjdk.java.net/jdk/jdk/file/9b4717ca9bd1/src/hotspot/share/oops/oopsHierarchy.cpp#l36
[4] http://hg.openjdk.java.net/jdk/jdk/file/9b4717ca9bd1/src/hotspot/os_cpu/linux_arm/os_linux_arm.cpp#l224
[5] http://hg.openjdk.java.net/jdk/jdk/file/9b4717ca9bd1/src/hotspot/cpu/arm/c1_LIRGenerator_arm.cpp#l1309
[6] http://hg.openjdk.java.net/jdk/jdk/file/9b4717ca9bd1/src/hotspot/cpu/arm/c1_LIRGenerator_arm.cpp#l1321
More information about the hotspot-runtime-dev
mailing list