Fix iterating over instructions following a store
Optimizer::eliminateLoadsFollowingSingleStore() was only looking at a
single instruction following the store.
Subzero derives Ice::Inst from llvm::ilist_node<Inst>, making it a node
in a circular linked list. However, we can't iterate until the end
because by default ilist_node<> does not provide sentinel node tracking.
If the llvm::ilist_node<Inst, llvm::ilist_sentinel_tracking<true>> class
is used instead, we can use isSentinel(), but this comes at the cost of
each node itself having to indicate whether it's the sentinel, and
keeping that up to date.
This change instead obtains the basic block's instruction list from the
store instruction, to get the iterator of the end node.
Bug: b/179668593
Change-Id: I994a7b37d8872380d2668c098d85000120d3e47a
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/52753
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
diff --git a/src/Reactor/Optimizer.cpp b/src/Reactor/Optimizer.cpp
index a3901d7..8be37b5 100644
--- a/src/Reactor/Optimizer.cpp
+++ b/src/Reactor/Optimizer.cpp
@@ -297,8 +297,13 @@
Ice::Inst *store = addressUses.stores[0];
Ice::Operand *storeValue = store->getData();
- for(Ice::Inst *load = &*++store->getIterator(), *next = nullptr; load != next; next = load, load = &*++store->getIterator())
+ auto instIterator = store->getIterator();
+ auto basicBlockEnd = getNode(store)->getInsts().end();
+
+ while(++instIterator != basicBlockEnd)
{
+ Ice::Inst *load = &*instIterator;
+
if(load->isDeleted() || !isLoad(*load))
{
continue;
diff --git a/tests/ReactorUnitTests/ReactorUnitTests.cpp b/tests/ReactorUnitTests/ReactorUnitTests.cpp
index aa91dbe..b6c0d9cf 100644
--- a/tests/ReactorUnitTests/ReactorUnitTests.cpp
+++ b/tests/ReactorUnitTests/ReactorUnitTests.cpp
@@ -376,6 +376,34 @@
EXPECT_EQ(result, 7);
}
+// This test excercises the Optimizer::eliminateLoadsFollowingSingleStore() optimization pass.
+// The three load operations for `y` should get eliminated.
+// TODO(b/180665600): Check that the optimization took place.
+TEST(ReactorUnitTests, EliminateLoadsFollowingSingleStore)
+{
+ FunctionT<int(int)> function;
+ {
+ Int x = function.Arg<0>();
+
+ Int y;
+ Int z;
+
+ // This branch materializes the variables.
+ If(x != 0) // TODO(b/179922668): Support If(x)
+ {
+ y = x;
+ z = y + y + y;
+ }
+
+ Return(z);
+ }
+
+ auto routine = function(testName().c_str());
+
+ int result = routine(11);
+ EXPECT_EQ(result, 33);
+}
+
// This test excercises the Optimizer::propagateAlloca() optimization pass.
// The pointer variable should not get stored to / loaded from memory.
// TODO(b/180665600): Check that the optimization took place.
