Change gtExtractSideEffList to use GenTreeVisitor (#18257)
authormikedn <onemihaid@hotmail.com>
Mon, 23 Jul 2018 19:21:34 +0000 (22:21 +0300)
committerEugene Rozenfeld <erozen@microsoft.com>
Mon, 23 Jul 2018 19:21:34 +0000 (12:21 -0700)
commit1f28125ad1f9975fbe68dd6839908aa6e63fc43b
treed44a8bcdd62a73123e5decfe501f22a3eeec1432
parent0c7a59efa088c2043ec57d7449f9f11dc96a395e
Change gtExtractSideEffList to use GenTreeVisitor (#18257)

* Change gtExtractSideEffList to use GenTreeVisitor

The tree traversal logic in gtExtractSideEffList is basically an incomplete duplicate of GenTreeVisitor's traversal logic. It lacks handling for GT_ARR_ELEM and GT_ARR_OFFSET so if this are encountered any side effects their children may have are silently dropped.

In addition, side effect ordering was not always preserved. The comma list is built by prepending nodes to it so side effects have to be visited in reverse execution order. The old code did this only for simple opers, for special nodes such as GT_ARR_BOUNDS_CHECK normal execution order was used.

This actually complicates a bit the GenTreeVisitor implementation as side effects need to be first stored in an array. The number of side effects is usually small (<= 4) so this shouldn't be a problem.

* Use GTF_SIDE_EFFECT in optPrepareTreeForReplacement

Assertion propagation doesn't have any way to ensure that it is safe to remove class constructor calls so GTF_PERSISTENT_SIDE_EFFECTS_IN_CSE should not be used here.

Likewise, GTF_EXCEPT side effects must be preserved as well. VN doesn't always propagate exceptions so it's possible to end up with a tree that has a constant VN and contains exception side effects.

* Add tests for 18232

* CR feedback
src/jit/assertionprop.cpp
src/jit/compmemkind.h
src/jit/gentree.cpp
tests/src/JIT/Regression/JitBlue/GitHub_18232/GitHub_18232.cs [new file with mode: 0644]
tests/src/JIT/Regression/JitBlue/GitHub_18232/GitHub_18232.csproj [new file with mode: 0644]