diff options
author | 2018-09-06 18:13:40 +0000 | |
---|---|---|
committer | 2018-09-06 19:44:51 +0100 | |
commit | d9986ac5946c6fb1fe10b754f6ca0f7e2f098382 (patch) | |
tree | 1be30c8d9d6e0528cb433a33db8eb07a4a6ed94d | |
parent | 0e840278fb2914678c648021950da9b5d4019468 (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.cc | 16 | ||||
-rw-r--r-- | compiler/optimizing/intrinsics.h | 1 | ||||
-rw-r--r-- | compiler/optimizing/intrinsics_arm64.cc | 7 | ||||
-rw-r--r-- | compiler/optimizing/intrinsics_arm_vixl.cc | 7 | ||||
-rw-r--r-- | compiler/optimizing/intrinsics_mips.cc | 7 | ||||
-rw-r--r-- | compiler/optimizing/intrinsics_mips64.cc | 7 | ||||
-rw-r--r-- | compiler/optimizing/intrinsics_x86.cc | 7 | ||||
-rw-r--r-- | compiler/optimizing/intrinsics_x86_64.cc | 7 | ||||
-rw-r--r-- | test/669-moveable-string-class-equals/expected.txt | 1 | ||||
-rw-r--r-- | test/669-moveable-string-class-equals/info.txt | 2 | ||||
-rwxr-xr-x | test/669-moveable-string-class-equals/run | 19 | ||||
-rw-r--r-- | test/669-moveable-string-class-equals/src/Main.java | 82 | ||||
-rw-r--r-- | test/common/runtime_state.cc | 9 | ||||
-rw-r--r-- | test/knownfailures.json | 5 |
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."] } ] |