Skip to content

Commit

Permalink
[JIT] Cleanup to lsra inspired by #73424 (#76481)
Browse files Browse the repository at this point in the history
Inspired by comments ([here](#73424 (comment)) and [here](
#73424 (comment)))

- Remove `RefPosition* currentRefPosition = &reverseIterator` as it creates two variables that are essentially the same, especially since the iterator has operators like `->`.  In fact, I had to spend a bit of time verifying that there weren't any updates to one but not the other in the code.  (And actually #73424 did slightly impact this relationship since it moved the assignment from the loop iteration step to the loop body, but I don't believe that this mattered.)
I renamed the "iterator" variable to the "position" name to reduce textual churn in the code.  This didn't work as well for the range-based loops since they yield -references- to the underlying object so a bunch of C++ punctuation changes.
- Break apart two complicated `for` conditions and remove duplication between the condition and the loop body.  Search for `continueLoop` to see them.
 - Delete list's `operator&` on each iterator since they are no longer used and not part of normal iterators.

Manually verified no diffs (other than memory addresses) on a jitdump for a test case with minopts on and off to exercise the dump code in lsra.  Manually verified asm diffs were spurious (differences in runtime addresses, sometimes also causing changes in the compiler internal representation size and then locations of labels).

This regains the unexplained small throughput loss in #73424.
  • Loading branch information
markples authored Oct 11, 2022
1 parent a7252b7 commit 20d4e30
Show file tree
Hide file tree
Showing 2 changed files with 159 additions and 188 deletions.
28 changes: 0 additions & 28 deletions src/coreclr/jit/jitstd/list.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ class list
bool operator==(const const_iterator& it) const;
bool operator!=(const const_iterator& it) const;
const T& operator*() const;
const T* operator&() const;
const T* operator->() const;
operator const T*() const;

Expand All @@ -85,7 +84,6 @@ class list
bool operator==(const iterator& it);
bool operator!=(const iterator& it);
T& operator*();
T* operator&();
T* operator->();
operator T*();

Expand Down Expand Up @@ -115,7 +113,6 @@ class list
bool operator==(const const_reverse_iterator& it) const;
bool operator!=(const const_reverse_iterator& it) const;
const T& operator*() const;
const T* operator&() const;
const T* operator->() const;
operator const T*() const;

Expand All @@ -142,7 +139,6 @@ class list
bool operator==(const reverse_iterator& it);
bool operator!=(const reverse_iterator& it);
T& operator*();
T* operator&();
T* operator->();
operator T*();
friend class list<T, Allocator>::const_reverse_iterator;
Expand Down Expand Up @@ -970,12 +966,6 @@ T& list<T, Allocator>::iterator::operator*()
return m_pNode->m_value;
}

template <typename T, typename Allocator>
T* list<T, Allocator>::iterator::operator&()
{
return &(m_pNode->m_value);
}

template <typename T, typename Allocator>
T* list<T, Allocator>::iterator::operator->()
{
Expand Down Expand Up @@ -1062,12 +1052,6 @@ const T& list<T, Allocator>::const_iterator::operator*() const
return m_pNode->m_value;
}

template <typename T, typename Allocator>
const T* list<T, Allocator>::const_iterator::operator&() const
{
return &(m_pNode->m_value);
}

template <typename T, typename Allocator>
const T* list<T, Allocator>::const_iterator::operator->() const
{
Expand Down Expand Up @@ -1147,12 +1131,6 @@ T& list<T, Allocator>::reverse_iterator::operator*()
return m_pNode->m_value;
}

template <typename T, typename Allocator>
T* list<T, Allocator>::reverse_iterator::operator&()
{
return &(m_pNode->m_value);
}

template <typename T, typename Allocator>
T* list<T, Allocator>::reverse_iterator::operator->()
{
Expand Down Expand Up @@ -1236,12 +1214,6 @@ const T& list<T, Allocator>::const_reverse_iterator::operator*() const
return m_pNode->m_value;
}

template <typename T, typename Allocator>
const T* list<T, Allocator>::const_reverse_iterator::operator&() const
{
return &(m_pNode->m_value);
}

template <typename T, typename Allocator>
const T* list<T, Allocator>::const_reverse_iterator::operator->() const
{
Expand Down
Loading

0 comments on commit 20d4e30

Please sign in to comment.