From 915b9d0c13bb5091875d868fbfa551d7b65d7477 Mon Sep 17 00:00:00 2001 From: Nicolas Geoffray Date: Wed, 11 Mar 2015 15:11:19 +0000 Subject: [PATCH] Tweak liveness when instructions are used in environments. Instructions remain live when debuggable, but only instructions with object types remain live when non-debuggable. Enable StackVisitor::GetThisObject for optimizing. Change-Id: Id87b2cbf33a02450059acc9993995782e5f28987 --- compiler/optimizing/code_generator.cc | 5 ++ compiler/optimizing/nodes.h | 1 + compiler/optimizing/ssa_builder.h | 4 +- compiler/optimizing/ssa_liveness_analysis.cc | 5 +- compiler/optimizing/ssa_liveness_analysis.h | 24 +++++- runtime/stack.cc | 6 -- test/461-get-reference-vreg/expected.txt | 0 .../get_reference_vreg_jni.cc | 86 ++++++++++++++++++++++ test/461-get-reference-vreg/info.txt | 1 + test/461-get-reference-vreg/src/Main.java | 65 ++++++++++++++++ test/Android.libarttest.mk | 3 +- test/Android.run-test.mk | 3 + 12 files changed, 192 insertions(+), 11 deletions(-) create mode 100644 test/461-get-reference-vreg/expected.txt create mode 100644 test/461-get-reference-vreg/get_reference_vreg_jni.cc create mode 100644 test/461-get-reference-vreg/info.txt create mode 100644 test/461-get-reference-vreg/src/Main.java diff --git a/compiler/optimizing/code_generator.cc b/compiler/optimizing/code_generator.cc index ed3f949af..38fb051f6 100644 --- a/compiler/optimizing/code_generator.cc +++ b/compiler/optimizing/code_generator.cc @@ -695,6 +695,11 @@ void CodeGenerator::RecordPcInfo(HInstruction* instruction, uint32_t dex_pc) { break; } + case Location::kInvalid: { + stack_map_stream_.AddDexRegisterEntry(DexRegisterMap::kNone, 0); + break; + } + default: LOG(FATAL) << "Unexpected kind " << location.GetKind(); } diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h index d4498a6d4..55639d437 100644 --- a/compiler/optimizing/nodes.h +++ b/compiler/optimizing/nodes.h @@ -1104,6 +1104,7 @@ class HInstruction : public ArenaObject { bool HasUses() const { return !uses_.IsEmpty() || !env_uses_.IsEmpty(); } bool HasEnvironmentUses() const { return !env_uses_.IsEmpty(); } + bool HasNonEnvironmentUses() const { return !uses_.IsEmpty(); } // Does this instruction strictly dominate `other_instruction`? // Returns false if this instruction and `other_instruction` are the same. diff --git a/compiler/optimizing/ssa_builder.h b/compiler/optimizing/ssa_builder.h index b414fb294..24dc44951 100644 --- a/compiler/optimizing/ssa_builder.h +++ b/compiler/optimizing/ssa_builder.h @@ -33,7 +33,9 @@ static constexpr int kDefaultNumberOfLoops = 2; * * (a) Dex registers that do not require merging (that is, they do not * have different values at a join block) are available to all their - * environment uses. + * environment uses. Note that it does not imply the instruction will + * have a physical location after register allocation. See the + * SsaLivenessAnalysis phase. * * (b) Dex registers that require merging, and the merging gives * incompatible types, will be killed for environment uses of that merge. diff --git a/compiler/optimizing/ssa_liveness_analysis.cc b/compiler/optimizing/ssa_liveness_analysis.cc index d009390a0..c0d6f42ca 100644 --- a/compiler/optimizing/ssa_liveness_analysis.cc +++ b/compiler/optimizing/ssa_liveness_analysis.cc @@ -230,11 +230,12 @@ void SsaLivenessAnalysis::ComputeLiveRanges() { } if (current->HasEnvironment()) { - // All instructions in the environment must be live. + // Handle environment uses. See statements (b) and (c) of the + // SsaLivenessAnalysis. HEnvironment* environment = current->GetEnvironment(); for (size_t i = 0, e = environment->Size(); i < e; ++i) { HInstruction* instruction = environment->GetInstructionAt(i); - if (instruction != nullptr) { + if (ShouldBeLiveForEnvironment(instruction)) { DCHECK(instruction->HasSsaIndex()); live_in->SetBit(instruction->GetSsaIndex()); instruction->GetLiveInterval()->AddUse(current, i, true); diff --git a/compiler/optimizing/ssa_liveness_analysis.h b/compiler/optimizing/ssa_liveness_analysis.h index 5787f0cc4..b57029d1a 100644 --- a/compiler/optimizing/ssa_liveness_analysis.h +++ b/compiler/optimizing/ssa_liveness_analysis.h @@ -302,7 +302,7 @@ class LiveInterval : public ArenaObject { first_range_->start_ = from; } else { // Instruction without uses. - DCHECK(!defined_by_->HasUses()); + DCHECK(!defined_by_->HasNonEnvironmentUses()); DCHECK(from == defined_by_->GetLifetimePosition()); first_range_ = last_range_ = new (allocator_) LiveRange(from, from + 2, nullptr); } @@ -798,6 +798,22 @@ class LiveInterval : public ArenaObject { DISALLOW_COPY_AND_ASSIGN(LiveInterval); }; +/** + * Analysis that computes the liveness of instructions: + * + * (a) Non-environment uses of an instruction always make + * the instruction live. + * (b) Environment uses of an instruction whose type is + * object (that is, non-primitive), make the instruction live. + * This is due to having to keep alive objects that have + * finalizers deleting native objects. + * (c) When the graph has the debuggable property, environment uses + * of an instruction that has a primitive type make the instruction live. + * If the graph does not have the debuggable property, the environment + * use has no effect, and may get a 'none' value after register allocation. + * + * (b) and (c) are implemented through SsaLivenessAnalysis::ShouldBeLiveForEnvironment. + */ class SsaLivenessAnalysis : public ValueObject { public: SsaLivenessAnalysis(const HGraph& graph, CodeGenerator* codegen) @@ -882,6 +898,12 @@ class SsaLivenessAnalysis : public ValueObject { // Update the live_out set of the block and returns whether it has changed. bool UpdateLiveOut(const HBasicBlock& block); + static bool ShouldBeLiveForEnvironment(HInstruction* instruction) { + if (instruction == nullptr) return false; + if (instruction->GetBlock()->GetGraph()->IsDebuggable()) return true; + return instruction->GetType() == Primitive::kPrimNot; + } + const HGraph& graph_; CodeGenerator* const codegen_; GrowableArray linear_order_; diff --git a/runtime/stack.cc b/runtime/stack.cc index 48becf688..24f71ed38 100644 --- a/runtime/stack.cc +++ b/runtime/stack.cc @@ -129,12 +129,6 @@ mirror::Object* StackVisitor::GetThisObject() const { } else { return cur_shadow_frame_->GetVRegReference(0); } - } else if (m->IsOptimized(GetInstructionSetPointerSize( - Runtime::Current()->GetInstructionSet()))) { - // TODO: Implement, currently only used for exceptions when jdwp is enabled. - UNIMPLEMENTED(WARNING) - << "StackVisitor::GetThisObject is unimplemented with the optimizing compiler"; - return nullptr; } else { const DexFile::CodeItem* code_item = m->GetCodeItem(); if (code_item == nullptr) { diff --git a/test/461-get-reference-vreg/expected.txt b/test/461-get-reference-vreg/expected.txt new file mode 100644 index 000000000..e69de29bb diff --git a/test/461-get-reference-vreg/get_reference_vreg_jni.cc b/test/461-get-reference-vreg/get_reference_vreg_jni.cc new file mode 100644 index 000000000..f0b78e1f5 --- /dev/null +++ b/test/461-get-reference-vreg/get_reference_vreg_jni.cc @@ -0,0 +1,86 @@ +/* + * Copyright (C) 2015 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. + */ + +#include "arch/context.h" +#include "jni.h" +#include "mirror/art_method-inl.h" +#include "scoped_thread_state_change.h" +#include "stack.h" +#include "thread.h" + +namespace art { + +namespace { + +class TestVisitor : public StackVisitor { + public: + TestVisitor(Thread* thread, Context* context, mirror::Object* this_value) + SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) + : StackVisitor(thread, context), this_value_(this_value), found_method_index_(0) {} + + bool VisitFrame() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { + mirror::ArtMethod* m = GetMethod(); + std::string m_name(m->GetName()); + + if (m_name.compare("testThisWithInstanceCall") == 0) { + found_method_index_ = 1; + uint32_t value = 0; + CHECK(GetVReg(m, 1, kReferenceVReg, &value)); + CHECK_EQ(reinterpret_cast(value), this_value_); + CHECK_EQ(GetThisObject(), this_value_); + } else if (m_name.compare("testThisWithStaticCall") == 0) { + found_method_index_ = 2; + uint32_t value = 0; + CHECK(GetVReg(m, 1, kReferenceVReg, &value)); + } else if (m_name.compare("testParameter") == 0) { + found_method_index_ = 3; + uint32_t value = 0; + CHECK(GetVReg(m, 1, kReferenceVReg, &value)); + } else if (m_name.compare("testObjectInScope") == 0) { + found_method_index_ = 4; + uint32_t value = 0; + CHECK(GetVReg(m, 0, kReferenceVReg, &value)); + } + + return true; + } + + mirror::Object* this_value_; + + // Value returned to Java to ensure the methods testSimpleVReg and testPairVReg + // have been found and tested. + jint found_method_index_; +}; + +extern "C" JNIEXPORT jint JNICALL Java_Main_doNativeCallRef(JNIEnv*, jobject value) { + ScopedObjectAccess soa(Thread::Current()); + std::unique_ptr context(Context::Create()); + TestVisitor visitor(soa.Self(), context.get(), soa.Decode(value)); + visitor.WalkStack(); + return visitor.found_method_index_; +} + +extern "C" JNIEXPORT jint JNICALL Java_Main_doStaticNativeCallRef(JNIEnv*, jclass) { + ScopedObjectAccess soa(Thread::Current()); + std::unique_ptr context(Context::Create()); + TestVisitor visitor(soa.Self(), context.get(), nullptr); + visitor.WalkStack(); + return visitor.found_method_index_; +} + +} // namespace + +} // namespace art diff --git a/test/461-get-reference-vreg/info.txt b/test/461-get-reference-vreg/info.txt new file mode 100644 index 000000000..1e5e97123 --- /dev/null +++ b/test/461-get-reference-vreg/info.txt @@ -0,0 +1 @@ +Tests for inspecting DEX registers holding references. diff --git a/test/461-get-reference-vreg/src/Main.java b/test/461-get-reference-vreg/src/Main.java new file mode 100644 index 000000000..a94c6fb38 --- /dev/null +++ b/test/461-get-reference-vreg/src/Main.java @@ -0,0 +1,65 @@ +/* + * Copyright (C) 2015 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 Main() { + } + + int testThisWithInstanceCall() { + return doNativeCallRef(); + } + + int testThisWithStaticCall() { + return doStaticNativeCallRef(); + } + + static int testParameter(Object a) { + return doStaticNativeCallRef(); + } + + static int testObjectInScope() { + Object a = array[0]; + return doStaticNativeCallRef(); + } + + native int doNativeCallRef(); + static native int doStaticNativeCallRef(); + + static { + System.loadLibrary("arttest"); + } + + public static void main(String[] args) { + Main rm = new Main(); + if (rm.testThisWithInstanceCall() != 1) { + throw new Error("Expected 1"); + } + + if (rm.testThisWithStaticCall() != 2) { + throw new Error("Expected 2"); + } + + if (testParameter(new Object()) != 3) { + throw new Error("Expected 3"); + } + + if (testObjectInScope() != 4) { + throw new Error("Expected 4"); + } + } + + static Object[] array = new Object[] { new Object() }; +} diff --git a/test/Android.libarttest.mk b/test/Android.libarttest.mk index f4bab3f9f..0cafb06a2 100644 --- a/test/Android.libarttest.mk +++ b/test/Android.libarttest.mk @@ -30,7 +30,8 @@ LIBARTTEST_COMMON_SRC_FILES := \ 118-noimage-dex2oat/noimage-dex2oat.cc \ 454-get-vreg/get_vreg_jni.cc \ 455-set-vreg/set_vreg_jni.cc \ - 457-regs/regs_jni.cc + 457-regs/regs_jni.cc \ + 461-get-reference-vreg/get_reference_vreg_jni.cc ART_TARGET_LIBARTTEST_$(ART_PHONY_TEST_TARGET_SUFFIX) += $(ART_TARGET_TEST_OUT)/$(TARGET_ARCH)/libarttest.so ifdef TARGET_2ND_ARCH diff --git a/test/Android.run-test.mk b/test/Android.run-test.mk index c666d359f..d4eaf4c49 100644 --- a/test/Android.run-test.mk +++ b/test/Android.run-test.mk @@ -312,6 +312,7 @@ TEST_ART_BROKEN_NDEBUG_TESTS := \ 454-get-vreg \ 455-set-vreg \ 457-regs \ + 461-get-reference-vreg \ ifneq (,$(filter ndebug,$(RUN_TYPES))) ART_TEST_KNOWN_BROKEN += $(call all-run-test-names,$(TARGET_TYPES),ndebug,$(PREBUILD_TYPES), \ @@ -367,6 +368,8 @@ TEST_ART_BROKEN_OPTIMIZING_RUN_TESTS := # Tests that should fail when the optimizing compiler compiles them non-debuggable. TEST_ART_BROKEN_OPTIMIZING_NONDEBUGGABLE_RUN_TESTS := \ + 454-get-vreg \ + 455-set-vreg \ 457-regs \ ifneq (,$(filter optimizing,$(COMPILER_TYPES))) -- 2.11.0