Fix HInstruction::ReplaceInput(), allow no-op.
Allow HInstruction::ReplaceInput() to be called with
a `replacement` being the same as the old input and
do nothing in that case.
This is a follow-up to
https://android-review.googlesource.com/216923
where I erroneously assumed that it never happens.
Also adhere to the standard C++ std::forward_list<>
semantics in the single-element overload of
`IntrusiveForwardList<>::splice_after()`.
Bug: 28173563
Change-Id: I5cea14c212b1083f90ffe6b5b53324ad663d57d8
diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc
index 7c6e931..fe75451 100644
--- a/compiler/optimizing/nodes.cc
+++ b/compiler/optimizing/nodes.cc
@@ -1022,8 +1022,11 @@
void HInstruction::ReplaceInput(HInstruction* replacement, size_t index) {
HUserRecord<HInstruction*> input_use = InputRecordAt(index);
+ if (input_use.GetInstruction() == replacement) {
+ // Nothing to do.
+ return;
+ }
HUseList<HInstruction*>::iterator before_use_node = input_use.GetBeforeUseNode();
- DCHECK(input_use.GetInstruction() != replacement);
// Note: fixup_end remains valid across splice_after().
auto fixup_end =
replacement->uses_.empty() ? replacement->uses_.begin() : ++replacement->uses_.begin();
diff --git a/compiler/utils/intrusive_forward_list.h b/compiler/utils/intrusive_forward_list.h
index a90b0f5..ec2c087 100644
--- a/compiler/utils/intrusive_forward_list.h
+++ b/compiler/utils/intrusive_forward_list.h
@@ -221,6 +221,13 @@
}
// Splice the element after `i`.
void splice_after(const_iterator position, IntrusiveForwardList& src, const_iterator i) {
+ // The standard specifies that this version does nothing if `position == i`
+ // or `position == ++i`. We must handle the latter here because the overload
+ // `splice_after(position, src, first, last)` does not allow `position` inside
+ // the range `(first, last)`.
+ if (++const_iterator(i) == position) {
+ return;
+ }
const_iterator last = i;
std::advance(last, 2);
splice_after(position, src, i, last);
diff --git a/compiler/utils/intrusive_forward_list_test.cc b/compiler/utils/intrusive_forward_list_test.cc
index d40ff3a..517142e 100644
--- a/compiler/utils/intrusive_forward_list_test.cc
+++ b/compiler/utils/intrusive_forward_list_test.cc
@@ -369,8 +369,33 @@
check.assign({8});
ASSERT_LISTS_EQUAL(check, ifl1);
+
+ // Move the first element of ref1/ifl1 to the beginning of ref1/ifl1 (do nothing).
+ ref1.splice_after(ref1.before_begin(), ref1, ref1.before_begin());
+ ifl1.splice_after(ifl1.before_begin(), ifl1, ifl1.before_begin());
+ ASSERT_LISTS_EQUAL(ref1, ifl1);
+ ASSERT_LISTS_EQUAL(check, ifl1);
+
+ // Move the first element of ref2/ifl2 after itself (do nothing).
+ ref1.splice_after(ref1.begin(), ref1, ref1.before_begin());
+ ifl1.splice_after(ifl1.begin(), ifl1, ifl1.before_begin());
+ ASSERT_LISTS_EQUAL(ref1, ifl1);
+ ASSERT_LISTS_EQUAL(check, ifl1);
+
check.assign({ 1, 7, 7, 2, 3, 4, 5, 4 });
ASSERT_LISTS_EQUAL(check, ifl2);
+
+ // Move the first element of ref2/ifl2 to the beginning of ref2/ifl2 (do nothing).
+ ref2.splice_after(ref2.before_begin(), ref2, ref2.before_begin());
+ ifl2.splice_after(ifl2.before_begin(), ifl2, ifl2.before_begin());
+ ASSERT_LISTS_EQUAL(ref2, ifl2);
+ ASSERT_LISTS_EQUAL(check, ifl2);
+
+ // Move the first element of ref2/ifl2 after itself (do nothing).
+ ref2.splice_after(ref2.begin(), ref2, ref2.before_begin());
+ ifl2.splice_after(ifl2.begin(), ifl2, ifl2.before_begin());
+ ASSERT_LISTS_EQUAL(ref2, ifl2);
+ ASSERT_LISTS_EQUAL(check, ifl2);
}
TEST(IntrusiveForwardList, Remove) {