Test variable materialization edge cases
These cases were discovered through various attempts to elide more loads
and stores after a branch has already occurred.
Also implement operator!= for Pointer<T>, and remove the redundant
Nucleus:createPtrEQ() which both LLVM and Subzero support through
integer value comparison.
Bug: b/180131694
Change-Id: I27c86b4fcbe6e1740801e40cb6877629f01d6540
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/52929
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>
diff --git a/src/Reactor/LLVMReactor.cpp b/src/Reactor/LLVMReactor.cpp
index 506e5dc..bc214f3 100644
--- a/src/Reactor/LLVMReactor.cpp
+++ b/src/Reactor/LLVMReactor.cpp
@@ -1437,12 +1437,6 @@
return V(jit->builder->CreateBitCast(V(v), T(destType)));
}
-Value *Nucleus::createPtrEQ(Value *lhs, Value *rhs)
-{
- RR_DEBUG_INFO_UPDATE_LOC();
- return V(jit->builder->CreateICmpEQ(V(lhs), V(rhs)));
-}
-
Value *Nucleus::createICmpEQ(Value *lhs, Value *rhs)
{
RR_DEBUG_INFO_UPDATE_LOC();
diff --git a/src/Reactor/Nucleus.hpp b/src/Reactor/Nucleus.hpp
index 4c08ddd..4d2b003 100644
--- a/src/Reactor/Nucleus.hpp
+++ b/src/Reactor/Nucleus.hpp
@@ -276,7 +276,6 @@
static Value *createBitCast(Value *V, Type *destType);
// Compare instructions
- static Value *createPtrEQ(Value *lhs, Value *rhs);
static Value *createICmpEQ(Value *lhs, Value *rhs);
static Value *createICmpNE(Value *lhs, Value *rhs);
static Value *createICmpUGT(Value *lhs, Value *rhs);
diff --git a/src/Reactor/Reactor.hpp b/src/Reactor/Reactor.hpp
index 4871c6e..87bfffd 100644
--- a/src/Reactor/Reactor.hpp
+++ b/src/Reactor/Reactor.hpp
@@ -2504,7 +2504,13 @@
template<typename T>
RValue<Bool> operator==(const Pointer<T> &lhs, const Pointer<T> &rhs)
{
- return RValue<Bool>(Nucleus::createPtrEQ(lhs.loadValue(), rhs.loadValue()));
+ return RValue<Bool>(Nucleus::createICmpEQ(lhs.loadValue(), rhs.loadValue()));
+}
+
+template<typename T>
+RValue<Bool> operator!=(const Pointer<T> &lhs, const Pointer<T> &rhs)
+{
+ return RValue<Bool>(Nucleus::createICmpNE(lhs.loadValue(), rhs.loadValue()));
}
template<typename T>
diff --git a/src/Reactor/SubzeroReactor.cpp b/src/Reactor/SubzeroReactor.cpp
index 15d15e3..41c07c8 100644
--- a/src/Reactor/SubzeroReactor.cpp
+++ b/src/Reactor/SubzeroReactor.cpp
@@ -1740,12 +1740,6 @@
return V(result);
}
-Value *Nucleus::createPtrEQ(Value *lhs, Value *rhs)
-{
- RR_DEBUG_INFO_UPDATE_LOC();
- return createIntCompare(Ice::InstIcmp::Eq, lhs, rhs);
-}
-
Value *Nucleus::createICmpEQ(Value *lhs, Value *rhs)
{
RR_DEBUG_INFO_UPDATE_LOC();
diff --git a/tests/ReactorUnitTests/ReactorUnitTests.cpp b/tests/ReactorUnitTests/ReactorUnitTests.cpp
index 16c9fb4..b9eeb07 100644
--- a/tests/ReactorUnitTests/ReactorUnitTests.cpp
+++ b/tests/ReactorUnitTests/ReactorUnitTests.cpp
@@ -266,6 +266,8 @@
EXPECT_EQ(result, 9);
}
+// This test excercises modifying the value of a local variable through a
+// pointer to it.
TEST(ReactorUnitTests, VariableAddress)
{
FunctionT<int(int)> function;
@@ -284,6 +286,96 @@
EXPECT_EQ(result, 20);
}
+// This test exercises taking the address of a local varible at the end of a
+// loop and modifying its value through the pointer in the second iteration.
+TEST(ReactorUnitTests, LateVariableAddress)
+{
+ FunctionT<int(void)> function;
+ {
+ Pointer<Int> p = nullptr;
+ Int a = 0;
+
+ While(a == 0)
+ {
+ If(p != Pointer<Int>(nullptr))
+ {
+ *p = 1;
+ }
+
+ p = &a;
+ }
+
+ Return(a);
+ }
+
+ auto routine = function(testName().c_str());
+
+ int result = routine();
+ EXPECT_EQ(result, 1);
+}
+
+// This test checks that the value of a local variable which has been modified
+// though a pointer is correct at the point before its address is (statically)
+// obtained.
+TEST(ReactorUnitTests, LoadAfterIndirectStore)
+{
+ FunctionT<int(void)> function;
+ {
+ Pointer<Int> p = nullptr;
+ Int a = 0;
+ Int b = 0;
+
+ While(a == 0)
+ {
+ If(p != Pointer<Int>(nullptr))
+ {
+ *p = 1;
+ }
+
+ // `a` must be loaded from memory here, despite not statically knowing
+ // yet that its address will be taken below.
+ b = a + 5;
+
+ p = &a;
+ }
+
+ Return(b);
+ }
+
+ auto routine = function(testName().c_str());
+
+ int result = routine();
+ EXPECT_EQ(result, 6);
+}
+
+// This test checks that variables statically accessed after a Return statement
+// are still loaded, modified, and stored correctly.
+TEST(ReactorUnitTests, LoopAfterReturn)
+{
+ FunctionT<int(void)> function;
+ {
+ Int min = 100;
+ Int max = 200;
+
+ If(min > max)
+ {
+ Return(5);
+ }
+
+ While(min < max)
+ {
+ min++;
+ }
+
+ Return(7);
+ }
+
+ auto routine = function(testName().c_str());
+
+ int result = routine();
+ EXPECT_EQ(result, 7);
+}
+
TEST(ReactorUnitTests, SubVectorLoadStore)
{
FunctionT<int(void *, void *)> function;
