summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Roland Levillain <rpl@google.com> 2018-09-06 18:13:40 +0000
committer Roland Levillain <rpl@google.com> 2018-09-06 19:44:51 +0100
commitd9986ac5946c6fb1fe10b754f6ca0f7e2f098382 (patch)
tree1be30c8d9d6e0528cb433a33db8eb07a4a6ed94d
parent0e840278fb2914678c648021950da9b5d4019468 (diff)
Revert "Fix String.equals() for moveable String.class."
This reverts commit da283050a1a3ddbb7cefae3f36e8c8c1a6acedb7. Reason for revert: This CL is no longer needed with https://android-review.googlesource.com/737437 which makes the String class non-movable in all cases. Test: art/test.py Bug: 68181300 Bug: 67628039 Change-Id: I414abd79de1e1b0fc43dc3be412fc70598ef3044
-rw-r--r--compiler/optimizing/instruction_simplifier.cc16
-rw-r--r--compiler/optimizing/intrinsics.h1
-rw-r--r--compiler/optimizing/intrinsics_arm64.cc7
-rw-r--r--compiler/optimizing/intrinsics_arm_vixl.cc7
-rw-r--r--compiler/optimizing/intrinsics_mips.cc7
-rw-r--r--compiler/optimizing/intrinsics_mips64.cc7
-rw-r--r--compiler/optimizing/intrinsics_x86.cc7
-rw-r--r--compiler/optimizing/intrinsics_x86_64.cc7
-rw-r--r--test/669-moveable-string-class-equals/expected.txt1
-rw-r--r--test/669-moveable-string-class-equals/info.txt2
-rwxr-xr-xtest/669-moveable-string-class-equals/run19
-rw-r--r--test/669-moveable-string-class-equals/src/Main.java82
-rw-r--r--test/common/runtime_state.cc9
-rw-r--r--test/knownfailures.json5
14 files changed, 0 insertions, 177 deletions
diff --git a/compiler/optimizing/instruction_simplifier.cc b/compiler/optimizing/instruction_simplifier.cc
index 2757f7b719..bb96c211cb 100644
--- a/compiler/optimizing/instruction_simplifier.cc
+++ b/compiler/optimizing/instruction_simplifier.cc
@@ -2146,22 +2146,6 @@ void InstructionSimplifierVisitor::SimplifyStringEquals(HInvoke* instruction) {
ReferenceTypeInfo argument_rti = argument->GetReferenceTypeInfo();
if (argument_rti.IsValid() && argument_rti.IsStringClass()) {
optimizations.SetArgumentIsString();
- } else if (kUseReadBarrier) {
- DCHECK(instruction->GetResolvedMethod() != nullptr);
- DCHECK(instruction->GetResolvedMethod()->GetDeclaringClass()->IsStringClass() ||
- // Object.equals() can be devirtualized to String.equals().
- instruction->GetResolvedMethod()->GetDeclaringClass()->IsObjectClass());
- Runtime* runtime = Runtime::Current();
- // For AOT, we always assume that the boot image shall contain the String.class and
- // we do not need a read barrier for boot image classes as they are non-moveable.
- // For JIT, check if we actually have a boot image; if we do, the String.class
- // should also be non-moveable.
- if (runtime->IsAotCompiler() || runtime->GetHeap()->HasBootImageSpace()) {
- DCHECK(runtime->IsAotCompiler() ||
- !runtime->GetHeap()->IsMovableObject(
- instruction->GetResolvedMethod()->GetDeclaringClass()));
- optimizations.SetNoReadBarrierForStringClass();
- }
}
}
}
diff --git a/compiler/optimizing/intrinsics.h b/compiler/optimizing/intrinsics.h
index 06e2fbb355..2d93f234be 100644
--- a/compiler/optimizing/intrinsics.h
+++ b/compiler/optimizing/intrinsics.h
@@ -219,7 +219,6 @@ class StringEqualsOptimizations : public IntrinsicOptimizations {
INTRINSIC_OPTIMIZATION(ArgumentNotNull, 0);
INTRINSIC_OPTIMIZATION(ArgumentIsString, 1);
- INTRINSIC_OPTIMIZATION(NoReadBarrierForStringClass, 2);
private:
DISALLOW_COPY_AND_ASSIGN(StringEqualsOptimizations);
diff --git a/compiler/optimizing/intrinsics_arm64.cc b/compiler/optimizing/intrinsics_arm64.cc
index 1abfcb022b..7684dc79f2 100644
--- a/compiler/optimizing/intrinsics_arm64.cc
+++ b/compiler/optimizing/intrinsics_arm64.cc
@@ -1398,13 +1398,6 @@ static const char* GetConstString(HInstruction* candidate, uint32_t* utf16_lengt
}
void IntrinsicLocationsBuilderARM64::VisitStringEquals(HInvoke* invoke) {
- if (kEmitCompilerReadBarrier &&
- !StringEqualsOptimizations(invoke).GetArgumentIsString() &&
- !StringEqualsOptimizations(invoke).GetNoReadBarrierForStringClass()) {
- // No support for this odd case (String class is moveable, not in the boot image).
- return;
- }
-
LocationSummary* locations =
new (allocator_) LocationSummary(invoke, LocationSummary::kNoCall, kIntrinsified);
locations->SetInAt(0, Location::RequiresRegister());
diff --git a/compiler/optimizing/intrinsics_arm_vixl.cc b/compiler/optimizing/intrinsics_arm_vixl.cc
index 1127fb8191..bc59fcf50c 100644
--- a/compiler/optimizing/intrinsics_arm_vixl.cc
+++ b/compiler/optimizing/intrinsics_arm_vixl.cc
@@ -1458,13 +1458,6 @@ static const char* GetConstString(HInstruction* candidate, uint32_t* utf16_lengt
}
void IntrinsicLocationsBuilderARMVIXL::VisitStringEquals(HInvoke* invoke) {
- if (kEmitCompilerReadBarrier &&
- !StringEqualsOptimizations(invoke).GetArgumentIsString() &&
- !StringEqualsOptimizations(invoke).GetNoReadBarrierForStringClass()) {
- // No support for this odd case (String class is moveable, not in the boot image).
- return;
- }
-
LocationSummary* locations =
new (allocator_) LocationSummary(invoke, LocationSummary::kNoCall, kIntrinsified);
InvokeRuntimeCallingConventionARMVIXL calling_convention;
diff --git a/compiler/optimizing/intrinsics_mips.cc b/compiler/optimizing/intrinsics_mips.cc
index 771714bf41..6f7f5e49c1 100644
--- a/compiler/optimizing/intrinsics_mips.cc
+++ b/compiler/optimizing/intrinsics_mips.cc
@@ -1516,13 +1516,6 @@ void IntrinsicCodeGeneratorMIPS::VisitStringCompareTo(HInvoke* invoke) {
// boolean java.lang.String.equals(Object anObject)
void IntrinsicLocationsBuilderMIPS::VisitStringEquals(HInvoke* invoke) {
- if (kEmitCompilerReadBarrier &&
- !StringEqualsOptimizations(invoke).GetArgumentIsString() &&
- !StringEqualsOptimizations(invoke).GetNoReadBarrierForStringClass()) {
- // No support for this odd case (String class is moveable, not in the boot image).
- return;
- }
-
LocationSummary* locations =
new (allocator_) LocationSummary(invoke, LocationSummary::kNoCall, kIntrinsified);
locations->SetInAt(0, Location::RequiresRegister());
diff --git a/compiler/optimizing/intrinsics_mips64.cc b/compiler/optimizing/intrinsics_mips64.cc
index 4a1bd5b7b2..2eb252908c 100644
--- a/compiler/optimizing/intrinsics_mips64.cc
+++ b/compiler/optimizing/intrinsics_mips64.cc
@@ -1369,13 +1369,6 @@ void IntrinsicCodeGeneratorMIPS64::VisitStringCompareTo(HInvoke* invoke) {
// boolean java.lang.String.equals(Object anObject)
void IntrinsicLocationsBuilderMIPS64::VisitStringEquals(HInvoke* invoke) {
- if (kEmitCompilerReadBarrier &&
- !StringEqualsOptimizations(invoke).GetArgumentIsString() &&
- !StringEqualsOptimizations(invoke).GetNoReadBarrierForStringClass()) {
- // No support for this odd case (String class is moveable, not in the boot image).
- return;
- }
-
LocationSummary* locations =
new (allocator_) LocationSummary(invoke, LocationSummary::kNoCall, kIntrinsified);
locations->SetInAt(0, Location::RequiresRegister());
diff --git a/compiler/optimizing/intrinsics_x86.cc b/compiler/optimizing/intrinsics_x86.cc
index d33c0c344e..3504d7a6f8 100644
--- a/compiler/optimizing/intrinsics_x86.cc
+++ b/compiler/optimizing/intrinsics_x86.cc
@@ -922,13 +922,6 @@ void IntrinsicCodeGeneratorX86::VisitStringCompareTo(HInvoke* invoke) {
}
void IntrinsicLocationsBuilderX86::VisitStringEquals(HInvoke* invoke) {
- if (kEmitCompilerReadBarrier &&
- !StringEqualsOptimizations(invoke).GetArgumentIsString() &&
- !StringEqualsOptimizations(invoke).GetNoReadBarrierForStringClass()) {
- // No support for this odd case (String class is moveable, not in the boot image).
- return;
- }
-
LocationSummary* locations =
new (allocator_) LocationSummary(invoke, LocationSummary::kNoCall, kIntrinsified);
locations->SetInAt(0, Location::RequiresRegister());
diff --git a/compiler/optimizing/intrinsics_x86_64.cc b/compiler/optimizing/intrinsics_x86_64.cc
index ae889744ad..96f6eaaf33 100644
--- a/compiler/optimizing/intrinsics_x86_64.cc
+++ b/compiler/optimizing/intrinsics_x86_64.cc
@@ -1230,13 +1230,6 @@ void IntrinsicCodeGeneratorX86_64::VisitStringCompareTo(HInvoke* invoke) {
}
void IntrinsicLocationsBuilderX86_64::VisitStringEquals(HInvoke* invoke) {
- if (kEmitCompilerReadBarrier &&
- !StringEqualsOptimizations(invoke).GetArgumentIsString() &&
- !StringEqualsOptimizations(invoke).GetNoReadBarrierForStringClass()) {
- // No support for this odd case (String class is moveable, not in the boot image).
- return;
- }
-
LocationSummary* locations =
new (allocator_) LocationSummary(invoke, LocationSummary::kNoCall, kIntrinsified);
locations->SetInAt(0, Location::RequiresRegister());
diff --git a/test/669-moveable-string-class-equals/expected.txt b/test/669-moveable-string-class-equals/expected.txt
deleted file mode 100644
index 6a5618ebc6..0000000000
--- a/test/669-moveable-string-class-equals/expected.txt
+++ /dev/null
@@ -1 +0,0 @@
-JNI_OnLoad called
diff --git a/test/669-moveable-string-class-equals/info.txt b/test/669-moveable-string-class-equals/info.txt
deleted file mode 100644
index 1d3202ef46..0000000000
--- a/test/669-moveable-string-class-equals/info.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-Regression test for String.equals() intrinsic instanceof check
-when the String.class is moveable.
diff --git a/test/669-moveable-string-class-equals/run b/test/669-moveable-string-class-equals/run
deleted file mode 100755
index 7c74d8ca0e..0000000000
--- a/test/669-moveable-string-class-equals/run
+++ /dev/null
@@ -1,19 +0,0 @@
-#!/bin/bash
-#
-# Copyright (C) 2017 The Android Open Source Project
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-# http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-
-# Run without image, so that String.class is moveable.
-# Reduce heap size to force more frequent GCs.
-${RUN} --no-image --runtime-option -Xmx16m "$@"
diff --git a/test/669-moveable-string-class-equals/src/Main.java b/test/669-moveable-string-class-equals/src/Main.java
deleted file mode 100644
index d182d51a25..0000000000
--- a/test/669-moveable-string-class-equals/src/Main.java
+++ /dev/null
@@ -1,82 +0,0 @@
-/*
- * Copyright (C) 2017 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-public class Main {
- public static void main(String[] args) {
- System.loadLibrary(args[0]);
- if (!hasJit()) {
- // Make the test pass if not using JIT.
- return;
- }
- if (hasImage()) {
- throw new Error("The `run` script should prevent this test from running with an image!");
- }
- if (!isClassMoveable(String.class)) {
- throw new Error("String.class not moveable despite running without image!");
- }
-
- // Make sure the Main.test() is JIT-compiled and then call it.
- ensureJitCompiled(Main.class, "test");
- test();
- }
-
- public static void test() {
- int length = 5;
-
- // Hide the type of these strings in an Object array,
- // so that we treat them as Object for the String.equals() below.
- Object[] array = new Object[length];
- for (int i = 0; i != length; ++i) {
- array[i] = "V" + i;
- }
-
- // Continually check string equality between a newly allocated String and an
- // already allocated String with the same contents while allocating over 128MiB
- // memory (with heap size limited to 16MiB), ensuring we run GC and stress the
- // instanceof check in the String.equals() implementation.
- for (int count = 0; count != 128 * 1024; ++count) {
- for (int i = 0; i != length; ++i) {
- allocateAtLeast1KiB();
- assertTrue(("V" + i).equals(array[i]));
- }
- }
- }
-
- public static void allocateAtLeast1KiB() {
- // Give GC more work by allocating Object arrays.
- memory[allocationIndex] = new Object[1024 / 4];
- ++allocationIndex;
- if (allocationIndex == memory.length) {
- allocationIndex = 0;
- }
- }
-
- public static void assertTrue(boolean value) {
- if (!value) {
- throw new Error("Assertion failed!");
- }
- }
-
- private native static boolean hasJit();
- private native static boolean hasImage();
- private native static boolean isClassMoveable(Class<?> cls);
- private static native void ensureJitCompiled(Class<?> itf, String method_name);
-
- // We shall retain some allocated memory and release old allocations
- // so that the GC has something to do.
- public static Object[] memory = new Object[4096];
- public static int allocationIndex = 0;
-}
diff --git a/test/common/runtime_state.cc b/test/common/runtime_state.cc
index da79164f12..c9b789e169 100644
--- a/test/common/runtime_state.cc
+++ b/test/common/runtime_state.cc
@@ -292,15 +292,6 @@ extern "C" JNIEXPORT void JNICALL Java_Main_fetchProfiles(JNIEnv*, jclass) {
code_cache->GetProfiledMethods(unused_locations, unused_vector);
}
-extern "C" JNIEXPORT jboolean JNICALL Java_Main_isClassMoveable(JNIEnv*,
- jclass,
- jclass cls) {
- Runtime* runtime = Runtime::Current();
- ScopedObjectAccess soa(Thread::Current());
- ObjPtr<mirror::Class> klass = soa.Decode<mirror::Class>(cls);
- return runtime->GetHeap()->IsMovableObject(klass);
-}
-
extern "C" JNIEXPORT void JNICALL Java_Main_waitForCompilation(JNIEnv*, jclass) {
jit::Jit* jit = Runtime::Current()->GetJit();
if (jit != nullptr) {
diff --git a/test/knownfailures.json b/test/knownfailures.json
index 40b699ea70..6004f25285 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -1077,10 +1077,5 @@
"tests": ["566-polymorphic-inlining"],
"variant": "jit & debuggable",
"description": ["We do not inline with debuggable."]
- },
- {
- "tests": ["669-moveable-string-class-equals"],
- "bug": "b/68181300",
- "description": ["The String class is now always non-movable."]
}
]