Subzero: temp fix Float4 % Float4 (FRem) Subzero emits incorrect code for FRem operations on vectors. Replace with an emulated version that calls fmodf on each component for now. Bug: b/148139679 Change-Id: Ib541370b71b99649f8c73f982fd12cb3cde25e7f Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/40450 Presubmit-Ready: Antonio Maiorano <amaiorano@google.com> Reviewed-by: Nicolas Capens <nicolascapens@google.com> Tested-by: Antonio Maiorano <amaiorano@google.com> Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
diff --git a/src/Reactor/EmulatedReactor.cpp b/src/Reactor/EmulatedReactor.cpp index 8e93909..dc3d558 100644 --- a/src/Reactor/EmulatedReactor.cpp +++ b/src/Reactor/EmulatedReactor.cpp
@@ -224,5 +224,10 @@ return call4(log2f, x); } +RValue<Float4> FRem(RValue<Float4> lhs, RValue<Float4> rhs) +{ + return call4(fmodf, lhs, rhs); +} + } // namespace emulated } // namespace rr
diff --git a/src/Reactor/EmulatedReactor.hpp b/src/Reactor/EmulatedReactor.hpp index 10b9b2a..ccc6245 100644 --- a/src/Reactor/EmulatedReactor.hpp +++ b/src/Reactor/EmulatedReactor.hpp
@@ -48,6 +48,7 @@ RValue<Float4> Log(RValue<Float4> x); RValue<Float4> Exp2(RValue<Float4> x); RValue<Float4> Log2(RValue<Float4> x); +RValue<Float4> FRem(RValue<Float4> lhs, RValue<Float4> rhs); } // namespace emulated } // namespace rr
diff --git a/src/Reactor/LLVMReactor.cpp b/src/Reactor/LLVMReactor.cpp index 99c43de..7cdf460 100644 --- a/src/Reactor/LLVMReactor.cpp +++ b/src/Reactor/LLVMReactor.cpp
@@ -1632,6 +1632,11 @@ return V(jit->builder->CreateFRem(V(lhs), V(rhs))); } +RValue<Float4> operator%(RValue<Float4> lhs, RValue<Float4> rhs) +{ + return RValue<Float4>(Nucleus::createFRem(lhs.value, rhs.value)); +} + Value *Nucleus::createShl(Value *lhs, Value *rhs) { RR_DEBUG_INFO_UPDATE_LOC();
diff --git a/src/Reactor/Reactor.cpp b/src/Reactor/Reactor.cpp index 5cf3bce..33e1972 100644 --- a/src/Reactor/Reactor.cpp +++ b/src/Reactor/Reactor.cpp
@@ -4240,11 +4240,6 @@ return RValue<Float4>(Nucleus::createFDiv(lhs.value, rhs.value)); } -RValue<Float4> operator%(RValue<Float4> lhs, RValue<Float4> rhs) -{ - return RValue<Float4>(Nucleus::createFRem(lhs.value, rhs.value)); -} - RValue<Float4> operator+=(Float4 &lhs, RValue<Float4> rhs) { return lhs = lhs + rhs;
diff --git a/src/Reactor/ReactorUnitTests.cpp b/src/Reactor/ReactorUnitTests.cpp index a72cb1e..ffe71c4 100644 --- a/src/Reactor/ReactorUnitTests.cpp +++ b/src/Reactor/ReactorUnitTests.cpp
@@ -25,32 +25,63 @@ constexpr float PI = 3.141592653589793f; using float4 = float[4]; +using int4 = int[4]; -// Value type wrapper around a float4 -struct float4_value +// TODO: Move to Reactor.hpp +template<> +struct rr::CToReactor<int[4]> { - float4_value() = default; - explicit float4_value(float rep) + using type = Int4; + static Int4 cast(float[4]); +}; + +// Value type wrapper around a <type>[4] (i.e. float4, int4) +template<typename T> +struct type4_value +{ + using E = typename std::remove_pointer_t<std::decay_t<T>>; + + type4_value() = default; + explicit type4_value(E rep) : v{ rep, rep, rep, rep } {} - float4_value(float x, float y, float z, float w) + type4_value(E x, E y, E z, E w) : v{ x, y, z, w } {} - bool operator==(const float4_value &rhs) const + bool operator==(const type4_value &rhs) const { return std::equal(std::begin(v), std::end(v), rhs.v); } // For gtest printing - friend std::ostream &operator<<(std::ostream &os, const float4_value &value) + friend std::ostream &operator<<(std::ostream &os, const type4_value &value) { return os << "[" << value.v[0] << ", " << value.v[1] << ", " << value.v[2] << ", " << value.v[3] << "]"; } - float4 v; + T v; }; +using float4_value = type4_value<float4>; +using int4_value = type4_value<int4>; + +// Invoke a void(type4_value<T>*) routine on &v.v, returning wrapped result in v +template<typename RoutineType, typename T> +type4_value<T> invokeRoutine(RoutineType &routine, type4_value<T> v) +{ + routine(&v.v); + return v; +} + +// Invoke a void(type4_value<T>*, type4_value<T>*) routine on &v1.v, &v2.v returning wrapped result in v1 +template<typename RoutineType, typename T> +type4_value<T> invokeRoutine(RoutineType &routine, type4_value<T> v1, type4_value<T> v2) +{ + routine(&v1.v, &v2.v); + return v1; +} + // For gtest printing of pairs namespace std { template<typename T, typename U> @@ -60,22 +91,6 @@ } } // namespace std -// Invoke a void(float4*) routine on &v.v, returning wrapped result in v -template<typename RoutineType> -float4_value invokeRoutine(RoutineType &routine, float4_value v) -{ - routine(&v.v); - return v; -} - -// Invoke a void(float4*, float4*) routine on &v1.v, &v2.v returning wrapped result in v1 -template<typename RoutineType> -float4_value invokeRoutine(RoutineType &routine, float4_value v1, float4_value v2) -{ - routine(&v1.v, &v2.v); - return v1; -} - int reference(int *p, int y) { int x = p[-1]; @@ -2237,6 +2252,44 @@ EXPECT_EQ(result[3], 678); } +TEST(ReactorUnitTests, SRem) +{ + FunctionT<void(int4 *, int4 *)> function; + { + Pointer<Int4> a = function.Arg<0>(); + Pointer<Int4> b = function.Arg<1>(); + *a = *a % *b; + } + + auto routine = function("one"); + + int4_value result = invokeRoutine(routine, int4_value{ 10, 11, 12, 13 }, int4_value{ 3, 3, 3, 3 }); + int4_value expected = int4_value{ 10 % 3, 11 % 3, 12 % 3, 13 % 3 }; + EXPECT_FLOAT_EQ(result.v[0], expected.v[0]); + EXPECT_FLOAT_EQ(result.v[1], expected.v[1]); + EXPECT_FLOAT_EQ(result.v[2], expected.v[2]); + EXPECT_FLOAT_EQ(result.v[3], expected.v[3]); +} + +TEST(ReactorUnitTests, FRem) +{ + FunctionT<void(float4 *, float4 *)> function; + { + Pointer<Float4> a = function.Arg<0>(); + Pointer<Float4> b = function.Arg<1>(); + *a = *a % *b; + } + + auto routine = function("one"); + + float4_value result = invokeRoutine(routine, float4_value{ 10.1f, 11.2f, 12.3f, 13.4f }, float4_value{ 3.f, 3.f, 3.f, 3.f }); + float4_value expected = float4_value{ fmodf(10.1f, 3.f), fmodf(11.2f, 3.f), fmodf(12.3f, 3.f), fmodf(13.4f, 3.f) }; + EXPECT_FLOAT_EQ(result.v[0], expected.v[0]); + EXPECT_FLOAT_EQ(result.v[1], expected.v[1]); + EXPECT_FLOAT_EQ(result.v[2], expected.v[2]); + EXPECT_FLOAT_EQ(result.v[3], expected.v[3]); +} + int main(int argc, char **argv) { ::testing::InitGoogleTest(&argc, argv);
diff --git a/src/Reactor/SubzeroReactor.cpp b/src/Reactor/SubzeroReactor.cpp index 54dcf93..91de455 100644 --- a/src/Reactor/SubzeroReactor.cpp +++ b/src/Reactor/SubzeroReactor.cpp
@@ -898,7 +898,15 @@ Value *Nucleus::createFRem(Value *lhs, Value *rhs) { - return createArithmetic(Ice::InstArithmetic::Frem, lhs, rhs); + // TODO(b/148139679) Fix Subzero generating invalid code for FRem on vector types + // createArithmetic(Ice::InstArithmetic::Frem, lhs, rhs); + UNIMPLEMENTED("Nucleus::createFRem"); + return nullptr; +} + +RValue<Float4> operator%(RValue<Float4> lhs, RValue<Float4> rhs) +{ + return emulated::FRem(lhs, rhs); } Value *Nucleus::createShl(Value *lhs, Value *rhs)