commit b3d9a2adab8a1b1f12ea518f8f7378bc20511e34
author Antonio Maiorano <amaiorano@google.com> Fri Feb 14 14:38:04 2020 -0500
committer Antonio Maiorano <amaiorano@google.com> Mon Feb 17 14:39:14 2020 +0000
tree cc4778b21731ab10d28128520bbe603287694a0e
parent 481daed34f15a118d6897d719e70230117f19cfa

Subzero: fix another load from constant data

When I (hack) fixed the fact that Subzero fails to load from a constant
source because it interprets it as an offset, I did so by routing all
loads into sz::createLoad, and in there, making sure to bitcast constant
sources to variables. I thought this covered all loads, but it turns out
that one of our optimization passes optimizes stores by removing
intermediate stores. This would sometimes replace a variable source with
a constant one, so I applied a similar hack here to avoid doing replace
in this case.

Bug: b/148272103
Change-Id: I809688e2c79fa1b62918d3b484a8e2a9e601bd90
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/41269
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
Tested-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Nicolas Capens <nicolascapens@google.com>

src/Reactor/Optimizer.cpp

diff --git a/src/Reactor/Optimizer.cpp b/src/Reactor/Optimizer.cpp
index f97b81b..184ac75 100644
--- a/src/Reactor/Optimizer.cpp
+++ b/src/Reactor/Optimizer.cpp
@@ -252,6 +252,14 @@
 					continue;
 				}
 
+				// TODO(b/148272103): InstLoad assumes that a constant ptr is an offset, rather than an
+				// absolute address. We need to make sure we don't replace a variable with a constant
+				// on this load.
+				if(llvm::isa<Ice::Constant>(storeValue))
+				{
+					continue;
+				}
+
 				replace(load, storeValue);
 
 				for(size_t i = 0; i < addressUses.loads.size(); i++)
@@ -415,6 +423,14 @@
 						continue;
 					}
 
+					// TODO(b/148272103): InstLoad assumes that a constant ptr is an offset, rather than an
+					// absolute address. We need to make sure we don't replace a variable with a constant
+					// on this load.
+					if(llvm::isa<Ice::Constant>(storeValue))
+					{
+						continue;
+					}
+
 					replace(inst, storeValue);
 				}
 			}

src/Reactor/SubzeroReactor.cpp

diff --git a/src/Reactor/SubzeroReactor.cpp b/src/Reactor/SubzeroReactor.cpp
index fbbec49..f2dbdef 100644
--- a/src/Reactor/SubzeroReactor.cpp
+++ b/src/Reactor/SubzeroReactor.cpp
@@ -922,10 +922,10 @@
 		rr::optimize(currFunc);
 
 		currFunc->computeInOutEdges();
-		ASSERT(!currFunc->hasError());
+		ASSERT_MSG(!currFunc->hasError(), "%s", currFunc->getError().c_str());
 
 		currFunc->translate();
-		ASSERT(!currFunc->hasError());
+		ASSERT_MSG(!currFunc->hasError(), "%s", currFunc->getError().c_str());
 
 		currFunc->getAssembler<>()->setInternal(currFunc->getInternal());