Fix locations at environment uses.
We were too agressive in not recording environment uses
when the instruction was not of type object. We have to
record the use to the use list of an interval, but it should
not affect the live ranges of that interval.
Change-Id: Id16fb7cc06f14083766d408a345837793583b6ea
diff --git a/compiler/optimizing/register_allocator.cc b/compiler/optimizing/register_allocator.cc
index cf38bd3..aa1584a 100644
--- a/compiler/optimizing/register_allocator.cc
+++ b/compiler/optimizing/register_allocator.cc
@@ -1408,26 +1408,36 @@
// Walk over all uses covered by this interval, and update the location
// information.
- while (use != nullptr && use->GetPosition() <= current->GetEnd()) {
- LocationSummary* locations = use->GetUser()->GetLocations();
- if (use->GetIsEnvironment()) {
- locations->SetEnvironmentAt(use->GetInputIndex(), source);
- } else {
- Location expected_location = locations->InAt(use->GetInputIndex());
- // The expected (actual) location may be invalid in case the input is unused. Currently
- // this only happens for intrinsics.
- if (expected_location.IsValid()) {
- if (expected_location.IsUnallocated()) {
- locations->SetInAt(use->GetInputIndex(), source);
- } else if (!expected_location.IsConstant()) {
- AddInputMoveFor(interval->GetDefinedBy(), use->GetUser(), source, expected_location);
- }
- } else {
- DCHECK(use->GetUser()->IsInvoke());
- DCHECK(use->GetUser()->AsInvoke()->GetIntrinsic() != Intrinsics::kNone);
- }
+
+ LiveRange* range = current->GetFirstRange();
+ while (range != nullptr) {
+ while (use != nullptr && use->GetPosition() < range->GetStart()) {
+ DCHECK(use->GetIsEnvironment());
+ use = use->GetNext();
}
- use = use->GetNext();
+ while (use != nullptr && use->GetPosition() <= range->GetEnd()) {
+ DCHECK(current->Covers(use->GetPosition()) || (use->GetPosition() == range->GetEnd()));
+ LocationSummary* locations = use->GetUser()->GetLocations();
+ if (use->GetIsEnvironment()) {
+ locations->SetEnvironmentAt(use->GetInputIndex(), source);
+ } else {
+ Location expected_location = locations->InAt(use->GetInputIndex());
+ // The expected (actual) location may be invalid in case the input is unused. Currently
+ // this only happens for intrinsics.
+ if (expected_location.IsValid()) {
+ if (expected_location.IsUnallocated()) {
+ locations->SetInAt(use->GetInputIndex(), source);
+ } else if (!expected_location.IsConstant()) {
+ AddInputMoveFor(interval->GetDefinedBy(), use->GetUser(), source, expected_location);
+ }
+ } else {
+ DCHECK(use->GetUser()->IsInvoke());
+ DCHECK(use->GetUser()->AsInvoke()->GetIntrinsic() != Intrinsics::kNone);
+ }
+ }
+ use = use->GetNext();
+ }
+ range = range->GetNext();
}
// If the next interval starts just after this one, and has a register,
@@ -1503,7 +1513,15 @@
}
current = next_sibling;
} while (current != nullptr);
- DCHECK(use == nullptr);
+
+ if (kIsDebugBuild) {
+ // Following uses can only be environment uses. The location for
+ // these environments will be none.
+ while (use != nullptr) {
+ DCHECK(use->GetIsEnvironment());
+ use = use->GetNext();
+ }
+ }
}
void RegisterAllocator::ConnectSplitSiblings(LiveInterval* interval,
diff --git a/compiler/optimizing/ssa_liveness_analysis.cc b/compiler/optimizing/ssa_liveness_analysis.cc
index 0f3973e..95da6ef 100644
--- a/compiler/optimizing/ssa_liveness_analysis.cc
+++ b/compiler/optimizing/ssa_liveness_analysis.cc
@@ -218,6 +218,26 @@
current->GetLiveInterval()->SetFrom(current->GetLifetimePosition());
}
+ // Process the environment first, because we know their uses come after
+ // or at the same liveness position of inputs.
+ if (current->HasEnvironment()) {
+ // 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);
+ bool should_be_live = ShouldBeLiveForEnvironment(instruction);
+ if (should_be_live) {
+ DCHECK(instruction->HasSsaIndex());
+ live_in->SetBit(instruction->GetSsaIndex());
+ }
+ if (instruction != nullptr) {
+ instruction->GetLiveInterval()->AddUse(
+ current, i, /* is_environment */ true, should_be_live);
+ }
+ }
+ }
+
// All inputs of an instruction must be live.
for (size_t i = 0, e = current->InputCount(); i < e; ++i) {
HInstruction* input = current->InputAt(i);
@@ -225,21 +245,7 @@
// to be materialized.
if (input->HasSsaIndex()) {
live_in->SetBit(input->GetSsaIndex());
- input->GetLiveInterval()->AddUse(current, i, false);
- }
- }
-
- if (current->HasEnvironment()) {
- // 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 (ShouldBeLiveForEnvironment(instruction)) {
- DCHECK(instruction->HasSsaIndex());
- live_in->SetBit(instruction->GetSsaIndex());
- instruction->GetLiveInterval()->AddUse(current, i, true);
- }
+ input->GetLiveInterval()->AddUse(current, i, /* is_environment */ false);
}
}
}
diff --git a/compiler/optimizing/ssa_liveness_analysis.h b/compiler/optimizing/ssa_liveness_analysis.h
index bc78dc2..d2da84c 100644
--- a/compiler/optimizing/ssa_liveness_analysis.h
+++ b/compiler/optimizing/ssa_liveness_analysis.h
@@ -189,7 +189,10 @@
AddRange(position, position + 1);
}
- void AddUse(HInstruction* instruction, size_t input_index, bool is_environment) {
+ void AddUse(HInstruction* instruction,
+ size_t input_index,
+ bool is_environment,
+ bool keep_alive = false) {
// Set the use within the instruction.
size_t position = instruction->GetLifetimePosition() + 1;
LocationSummary* locations = instruction->GetLocations();
@@ -211,6 +214,7 @@
&& (first_use_->GetPosition() < position)) {
// The user uses the instruction multiple times, and one use dies before the other.
// We update the use list so that the latter is first.
+ DCHECK(!is_environment);
UsePosition* cursor = first_use_;
while ((cursor->GetNext() != nullptr) && (cursor->GetNext()->GetPosition() < position)) {
cursor = cursor->GetNext();
@@ -225,6 +229,15 @@
return;
}
+ first_use_ = new (allocator_) UsePosition(
+ instruction, input_index, is_environment, position, first_use_);
+
+ if (is_environment && !keep_alive) {
+ // If this environment use does not keep the instruction live, it does not
+ // affect the live range of that instruction.
+ return;
+ }
+
size_t start_block_position = instruction->GetBlock()->GetLifetimeStart();
if (first_range_ == nullptr) {
// First time we see a use of that interval.
@@ -246,8 +259,6 @@
// and the check line 205 would succeed.
first_range_ = new (allocator_) LiveRange(start_block_position, position, first_range_);
}
- first_use_ = new (allocator_) UsePosition(
- instruction, input_index, is_environment, position, first_use_);
}
void AddPhiUse(HInstruction* instruction, size_t input_index, HBasicBlock* block) {
@@ -425,9 +436,11 @@
UsePosition* use = first_use_;
size_t end = GetEnd();
while (use != nullptr && use->GetPosition() <= end) {
- size_t use_position = use->GetPosition();
- if (use_position > position) {
- return use_position;
+ if (!use->GetIsEnvironment()) {
+ size_t use_position = use->GetPosition();
+ if (use_position > position) {
+ return use_position;
+ }
}
use = use->GetNext();
}
diff --git a/test/466-get-live-vreg/expected.txt b/test/466-get-live-vreg/expected.txt
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/test/466-get-live-vreg/expected.txt
diff --git a/test/466-get-live-vreg/get_live_vreg_jni.cc b/test/466-get-live-vreg/get_live_vreg_jni.cc
new file mode 100644
index 0000000..6715ba1
--- /dev/null
+++ b/test/466-get-live-vreg/get_live_vreg_jni.cc
@@ -0,0 +1,71 @@
+/*
+ * 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) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
+ : StackVisitor(thread, context) {}
+
+ bool VisitFrame() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
+ mirror::ArtMethod* m = GetMethod();
+ std::string m_name(m->GetName());
+
+ if (m_name.compare("testLiveArgument") == 0) {
+ found_method_ = true;
+ uint32_t value = 0;
+ CHECK(GetVReg(m, 0, kIntVReg, &value));
+ CHECK_EQ(value, 42u);
+ } else if (m_name.compare("testIntervalHole") == 0) {
+ found_method_ = true;
+ uint32_t value = 0;
+ if (m->IsOptimized(sizeof(void*))) {
+ CHECK_EQ(GetVReg(m, 0, kIntVReg, &value), false);
+ } else {
+ CHECK(GetVReg(m, 0, kIntVReg, &value));
+ CHECK_EQ(value, 1u);
+ }
+ }
+
+ return true;
+ }
+
+ // Value returned to Java to ensure the methods testSimpleVReg and testPairVReg
+ // have been found and tested.
+ bool found_method_ = false;
+};
+
+extern "C" JNIEXPORT void JNICALL Java_Main_doStaticNativeCallLiveVreg(JNIEnv*, jclass) {
+ ScopedObjectAccess soa(Thread::Current());
+ std::unique_ptr<Context> context(Context::Create());
+ TestVisitor visitor(soa.Self(), context.get());
+ visitor.WalkStack();
+ CHECK(visitor.found_method_);
+}
+
+} // namespace
+
+} // namespace art
diff --git a/test/466-get-live-vreg/info.txt b/test/466-get-live-vreg/info.txt
new file mode 100644
index 0000000..cbe0c9a
--- /dev/null
+++ b/test/466-get-live-vreg/info.txt
@@ -0,0 +1,3 @@
+Tests for inspecting live DEX registers. The test must
+also pass for non-debuggable, as we need to know live DEX registers
+when de-opting.
diff --git a/test/466-get-live-vreg/src/Main.java b/test/466-get-live-vreg/src/Main.java
new file mode 100644
index 0000000..3118085
--- /dev/null
+++ b/test/466-get-live-vreg/src/Main.java
@@ -0,0 +1,70 @@
+/*
+ * 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() {
+ }
+
+ static int testLiveArgument(int arg) {
+ doStaticNativeCallLiveVreg();
+ return arg;
+ }
+
+ static void moveArgToCalleeSave() {
+ try {
+ Thread.sleep(0);
+ } catch (Exception e) {
+ throw new Error(e);
+ }
+ }
+
+ static void testIntervalHole(int arg, boolean test) {
+ // Move the argument to callee save to ensure it is in
+ // a readable register.
+ moveArgToCalleeSave();
+ if (test) {
+ staticField1 = arg;
+ // The environment use of `arg` should not make it live.
+ doStaticNativeCallLiveVreg();
+ } else {
+ staticField2 = arg;
+ // The environment use of `arg` should not make it live.
+ doStaticNativeCallLiveVreg();
+ }
+ }
+
+ static native void doStaticNativeCallLiveVreg();
+
+ static {
+ System.loadLibrary("arttest");
+ }
+
+ public static void main(String[] args) {
+ if (testLiveArgument(42) != 42) {
+ throw new Error("Expected 42");
+ }
+
+ if (testLiveArgument(42) != 42) {
+ throw new Error("Expected 42");
+ }
+
+ testIntervalHole(1, true);
+ testIntervalHole(1, false);
+ }
+
+ static int staticField1;
+ static int staticField2;
+}
diff --git a/test/Android.libarttest.mk b/test/Android.libarttest.mk
index 0cafb06..5e768ee 100644
--- a/test/Android.libarttest.mk
+++ b/test/Android.libarttest.mk
@@ -31,7 +31,8 @@
454-get-vreg/get_vreg_jni.cc \
455-set-vreg/set_vreg_jni.cc \
457-regs/regs_jni.cc \
- 461-get-reference-vreg/get_reference_vreg_jni.cc
+ 461-get-reference-vreg/get_reference_vreg_jni.cc \
+ 466-get-live-vreg/get_live_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 28fbc3e..39afc67 100644
--- a/test/Android.run-test.mk
+++ b/test/Android.run-test.mk
@@ -313,6 +313,7 @@
455-set-vreg \
457-regs \
461-get-reference-vreg \
+ 466-get-live-vreg \
ifneq (,$(filter ndebug,$(RUN_TYPES)))
ART_TEST_KNOWN_BROKEN += $(call all-run-test-names,$(TARGET_TYPES),ndebug,$(PREBUILD_TYPES), \
