Fix rr::Print and add unit tests
* CMake: add REACTOR_ENABLE_PRINT to enable rr::Print macros (default 0).
When enabled, defines ENABLE_RR_PRINT. Formerly, ENABLE_RR_PRINT was
only defined in NDEBUG builds.
* CMake: REACTOR_EMIT_PRINT_LOCATION enables REACTOR_ENABLE_PRINT.
* Add missing includes of Print.hpp.
* Fix rr::Print compilation of non-const pointers.
* Fix rr::Print compilation of Int2 and UInt2.
* Fix rr::Print of Float outputting in square brackets.
* Fix rr::Print of Bool to output a '1' or '0' for 'true' and 'false'.
* Add unit tests for rr::Print of all primitive and Reactor types.
* Includes changes bclayton@ made to output "true"/"false" for Bool,
and to correctly compare against implementation-specific printf of
pointers.
Bug: b/149328074
Change-Id: Ibb17985201f1c2b432539cd1153293d1e1cbb5bb
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/41108
Reviewed-by: Ben Clayton <bclayton@google.com>
Tested-by: Antonio Maiorano <amaiorano@google.com>
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 0ff9cd9..045df81 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -120,6 +120,7 @@
option_if_not_defined(SWIFTSHADER_DCHECK_ALWAYS_ON "Check validation macros even in release builds" 0)
option_if_not_defined(REACTOR_EMIT_DEBUG_INFO "Emit debug info for JIT functions" 0)
option_if_not_defined(REACTOR_EMIT_PRINT_LOCATION "Emit printing of location info for JIT functions" 0)
+option_if_not_defined(REACTOR_ENABLE_PRINT "Enable RR_PRINT macros" 0)
option_if_not_defined(REACTOR_VERIFY_LLVM_IR "Check reactor-generated LLVM IR is valid even in release builds" 0)
option_if_not_defined(SWIFTSHADER_LESS_DEBUG_INFO "Generate less debug info to reduce file size" 0)
option_if_not_defined(SWIFTSHADER_ENABLE_VULKAN_DEBUGGER "Enable vulkan debugger support" 0)
@@ -543,8 +544,9 @@
endif()
if(REACTOR_EMIT_PRINT_LOCATION)
- # This feature depends on REACTOR_EMIT_DEBUG_INFO, so enable it
+ # This feature depends on REACTOR_EMIT_DEBUG_INFO and REACTOR_ENABLE_PRINT
set(REACTOR_EMIT_DEBUG_INFO "On")
+ set(REACTOR_ENABLE_PRINT "On")
list(APPEND SWIFTSHADER_COMPILE_OPTIONS "-DENABLE_RR_EMIT_PRINT_LOCATION")
endif()
@@ -553,6 +555,10 @@
list(APPEND SWIFTSHADER_COMPILE_OPTIONS "-DENABLE_RR_DEBUG_INFO")
endif()
+if(REACTOR_ENABLE_PRINT)
+ list(APPEND SWIFTSHADER_COMPILE_OPTIONS "-DENABLE_RR_PRINT")
+endif()
+
if(REACTOR_VERIFY_LLVM_IR)
list(APPEND SWIFTSHADER_COMPILE_OPTIONS "-DENABLE_RR_LLVM_IR_VERIFICATION")
endif()
diff --git a/src/Reactor/LLVMReactor.cpp b/src/Reactor/LLVMReactor.cpp
index c8d74c2..c14012d 100644
--- a/src/Reactor/LLVMReactor.cpp
+++ b/src/Reactor/LLVMReactor.cpp
@@ -18,6 +18,7 @@
#include "Debug.hpp"
#include "EmulatedReactor.hpp"
#include "LLVMReactorDebugInfo.hpp"
+#include "Print.hpp"
#include "Reactor.hpp"
#include "x86.hpp"
@@ -3874,6 +3875,13 @@
return elements;
}
+std::vector<Value *> PrintValue::Ty<Bool>::val(const RValue<Bool> &v)
+{
+ auto t = jit->builder->CreateGlobalStringPtr("true");
+ auto f = jit->builder->CreateGlobalStringPtr("false");
+ return { V(jit->builder->CreateSelect(V(v.value), t, f)) };
+}
+
std::vector<Value *> PrintValue::Ty<Byte>::val(const RValue<Byte> &v)
{
return toInt({ v.value }, false);
@@ -3965,7 +3973,7 @@
if(function != nullptr) { str += "%s "; }
str += fmt;
- // Perform subsitution on all '{n}' bracketed indices in the format
+ // Perform substitution on all '{n}' bracketed indices in the format
// message.
int i = 0;
for(const PrintValue &arg : args)
diff --git a/src/Reactor/LLVMReactorDebugInfo.cpp b/src/Reactor/LLVMReactorDebugInfo.cpp
index 21a759c..5810d44 100644
--- a/src/Reactor/LLVMReactorDebugInfo.cpp
+++ b/src/Reactor/LLVMReactorDebugInfo.cpp
@@ -18,6 +18,7 @@
# include "LLVMReactor.hpp"
# include "Reactor.hpp"
+# include "Print.hpp"
# include "boost/stacktrace.hpp"
diff --git a/src/Reactor/Print.hpp b/src/Reactor/Print.hpp
index a0dee69..a5bde3e 100644
--- a/src/Reactor/Print.hpp
+++ b/src/Reactor/Print.hpp
@@ -15,10 +15,6 @@
#ifndef rr_Print_hpp
#define rr_Print_hpp
-#if !defined(NDEBUG)
-# define ENABLE_RR_PRINT 1 // Enables RR_PRINT(), RR_WATCH()
-#endif // !defined(NDEBUG)
-
#ifdef ENABLE_RR_PRINT
# include "Reactor.hpp"
@@ -147,9 +143,12 @@
PrintValue(double v)
: format(std::to_string(v))
{}
+ PrintValue(const char *v)
+ : format(v)
+ {}
template<typename T>
- PrintValue(const T *v)
+ PrintValue(T *v)
: format(addr(v))
{}
@@ -226,8 +225,8 @@
template<>
struct PrintValue::Ty<Bool>
{
- static std::string fmt(const RValue<Bool> &v) { return "%d"; }
- static std::vector<Value *> val(const RValue<Bool> &v) { return { v.value }; }
+ static std::string fmt(const RValue<Bool> &v) { return "%s"; }
+ static std::vector<Value *> val(const RValue<Bool> &v);
};
template<>
struct PrintValue::Ty<Byte>
@@ -250,7 +249,7 @@
template<>
struct PrintValue::Ty<Int2>
{
- static std::string fmt(const RValue<Int> &v) { return "[%d, %d]"; }
+ static std::string fmt(const RValue<Int2> &v) { return "[%d, %d]"; }
static std::vector<Value *> val(const RValue<Int2> &v);
};
template<>
@@ -268,7 +267,7 @@
template<>
struct PrintValue::Ty<UInt2>
{
- static std::string fmt(const RValue<UInt> &v) { return "[%u, %u]"; }
+ static std::string fmt(const RValue<UInt2> &v) { return "[%u, %u]"; }
static std::vector<Value *> val(const RValue<UInt2> &v);
};
template<>
@@ -304,7 +303,7 @@
template<>
struct PrintValue::Ty<Float>
{
- static std::string fmt(const RValue<Float> &v) { return "[%f]"; }
+ static std::string fmt(const RValue<Float> &v) { return "%f"; }
static std::vector<Value *> val(const RValue<Float> &v);
};
template<>
diff --git a/src/Reactor/ReactorUnitTests.cpp b/src/Reactor/ReactorUnitTests.cpp
index 508e815..63db739 100644
--- a/src/Reactor/ReactorUnitTests.cpp
+++ b/src/Reactor/ReactorUnitTests.cpp
@@ -13,6 +13,7 @@
// limitations under the License.
#include "Coroutine.hpp"
+#include "Print.hpp"
#include "Reactor.hpp"
#include "gtest/gtest.h"
@@ -107,6 +108,206 @@
return sum;
}
+class StdOutCapture
+{
+public:
+ ~StdOutCapture()
+ {
+ stopIfCapturing();
+ }
+
+ void start()
+ {
+ stopIfCapturing();
+ capturing = true;
+ testing::internal::CaptureStdout();
+ }
+
+ std::string stop()
+ {
+ assert(capturing);
+ capturing = false;
+ return testing::internal::GetCapturedStdout();
+ }
+
+private:
+ void stopIfCapturing()
+ {
+ if(capturing)
+ {
+ // This stops the capture
+ testing::internal::GetCapturedStdout();
+ }
+ }
+
+ bool capturing = false;
+};
+
+std::vector<std::string> split(const std::string &s)
+{
+ std::vector<std::string> result;
+ std::istringstream iss(s);
+ for(std::string line; std::getline(iss, line);)
+ {
+ result.push_back(line);
+ }
+ return result;
+}
+
+TEST(ReactorUnitTests, PrintPrimitiveTypes)
+{
+#ifdef ENABLE_RR_PRINT
+ FunctionT<void()> function;
+ {
+ bool b(true);
+ int8_t i8(-1);
+ uint8_t ui8(1);
+ int16_t i16(-1);
+ uint16_t ui16(1);
+ int32_t i32(-1);
+ uint32_t ui32(1);
+ int64_t i64(-1);
+ uint64_t ui64(1);
+ float f(1);
+ double d(2);
+ const char *cstr = "const char*";
+ std::string str = "std::string";
+ int *p = nullptr;
+
+ RR_WATCH(b);
+ RR_WATCH(i8);
+ RR_WATCH(ui8);
+ RR_WATCH(i16);
+ RR_WATCH(ui16);
+ RR_WATCH(i32);
+ RR_WATCH(ui32);
+ RR_WATCH(i64);
+ RR_WATCH(ui64);
+ RR_WATCH(f);
+ RR_WATCH(d);
+ RR_WATCH(cstr);
+ RR_WATCH(str);
+ RR_WATCH(p);
+ }
+
+ auto routine = function("one");
+
+ char pNullptr[64];
+ snprintf(pNullptr, sizeof(pNullptr), " p: %p", nullptr);
+
+ const char *expected[] = {
+ " b: true",
+ " i8: -1",
+ " ui8: 1",
+ " i16: -1",
+ " ui16: 1",
+ " i32: -1",
+ " ui32: 1",
+ " i64: -1",
+ " ui64: 1",
+ " f: 1.000000",
+ " d: 2.000000",
+ " cstr: const char*",
+ " str: std::string",
+ pNullptr,
+ };
+ constexpr size_t expectedSize = sizeof(expected) / sizeof(expected[0]);
+
+ StdOutCapture capture;
+ capture.start();
+ routine();
+ auto output = split(capture.stop());
+ for(size_t i = 0, j = 1; i < expectedSize; ++i, j += 2)
+ {
+ ASSERT_EQ(expected[i], output[j]);
+ }
+
+#endif
+}
+
+TEST(ReactorUnitTests, PrintReactorTypes)
+{
+#ifdef ENABLE_RR_PRINT
+ FunctionT<void()> function;
+ {
+ Bool b(true);
+ Int i(-1);
+ Int2 i2(-1, -2);
+ Int4 i4(-1, -2, -3, -4);
+ UInt ui(1);
+ UInt2 ui2(1, 2);
+ UInt4 ui4(1, 2, 3, 4);
+ Short s(-1);
+ Short4 s4(-1, -2, -3, -4);
+ UShort us(1);
+ UShort4 us4(1, 2, 3, 4);
+ Float f(1);
+ Float4 f4(1, 2, 3, 4);
+ Long l(i);
+ Pointer<Int> pi = nullptr;
+ RValue<Int> rvi = i;
+ Byte by('a');
+ Byte4 by4(i4);
+
+ RR_WATCH(b);
+ RR_WATCH(i);
+ RR_WATCH(i2);
+ RR_WATCH(i4);
+ RR_WATCH(ui);
+ RR_WATCH(ui2);
+ RR_WATCH(ui4);
+ RR_WATCH(s);
+ RR_WATCH(s4);
+ RR_WATCH(us);
+ RR_WATCH(us4);
+ RR_WATCH(f);
+ RR_WATCH(f4);
+ RR_WATCH(l);
+ RR_WATCH(pi);
+ RR_WATCH(rvi);
+ RR_WATCH(by);
+ RR_WATCH(by4);
+ }
+
+ auto routine = function("one");
+
+ char piNullptr[64];
+ snprintf(piNullptr, sizeof(piNullptr), " pi: %p", nullptr);
+
+ const char *expected[] = {
+ " b: true",
+ " i: -1",
+ " i2: [-1, -2]",
+ " i4: [-1, -2, -3, -4]",
+ " ui: 1",
+ " ui2: [1, 2]",
+ " ui4: [1, 2, 3, 4]",
+ " s: -1",
+ " s4: [-1, -2, -3, -4]",
+ " us: 1",
+ " us4: [1, 2, 3, 4]",
+ " f: 1.000000",
+ " f4: [1.000000, 2.000000, 3.000000, 4.000000]",
+ " l: -1",
+ piNullptr,
+ " rvi: -1",
+ " by: 97",
+ " by4: [255, 254, 253, 252]",
+ };
+ constexpr size_t expectedSize = sizeof(expected) / sizeof(expected[0]);
+
+ StdOutCapture capture;
+ capture.start();
+ routine();
+ auto output = split(capture.stop());
+ for(size_t i = 0, j = 1; i < expectedSize; ++i, j += 2)
+ {
+ ASSERT_EQ(expected[i], output[j]);
+ }
+
+#endif
+}
+
TEST(ReactorUnitTests, Sample)
{
FunctionT<int(int *, int)> function;
diff --git a/src/Shader/ShaderCore.hpp b/src/Shader/ShaderCore.hpp
index fd1ccb7..6a0b285 100644
--- a/src/Shader/ShaderCore.hpp
+++ b/src/Shader/ShaderCore.hpp
@@ -17,6 +17,7 @@
#include "Shader.hpp"
#include "Reactor/Reactor.hpp"
+#include "Reactor/Print.hpp"
#include "Common/Debug.hpp"
namespace sw
