Commit 3ab17bd7 by Nicolas Capens Committed by Nicolas Capens

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: 's avatarNicolas Capens <nicolascapens@google.com> Reviewed-by: 's avatarAntonio Maiorano <amaiorano@google.com>
parent 0b506e1d
...@@ -297,8 +297,13 @@ void Optimizer::eliminateLoadsFollowingSingleStore() ...@@ -297,8 +297,13 @@ void Optimizer::eliminateLoadsFollowingSingleStore()
Ice::Inst *store = addressUses.stores[0]; Ice::Inst *store = addressUses.stores[0];
Ice::Operand *storeValue = store->getData(); 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)) if(load->isDeleted() || !isLoad(*load))
{ {
continue; continue;
......
...@@ -376,6 +376,34 @@ TEST(ReactorUnitTests, LoopAfterReturn) ...@@ -376,6 +376,34 @@ TEST(ReactorUnitTests, LoopAfterReturn)
EXPECT_EQ(result, 7); 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. // This test excercises the Optimizer::propagateAlloca() optimization pass.
// The pointer variable should not get stored to / loaded from memory. // The pointer variable should not get stored to / loaded from memory.
// TODO(b/180665600): Check that the optimization took place. // TODO(b/180665600): Check that the optimization took place.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment