make hide addresses a runtime option

PhilippGrulich · PhilippGrulich · commit edd95efd2983 · 2024-10-08T21:07:17.000+02:00
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -73,7 +73,7 @@ jobs:
       - name: cmake
         shell: bash
         run: |
-          cmake -DCMAKE_BUILD_TYPE=Release -D CMAKE_C_COMPILER_LAUNCHER=ccache -DLOGGING_HIDE_ADDRESSES=ON -D CMAKE_CXX_COMPILER_LAUNCHER=ccache ${{ matrix.flags }}  -G Ninja -S . -B .
+          cmake -DCMAKE_BUILD_TYPE=Release -D CMAKE_C_COMPILER_LAUNCHER=ccache -D CMAKE_CXX_COMPILER_LAUNCHER=ccache ${{ matrix.flags }}  -G Ninja -S . -B .
       - name: build
         shell: bash
         run: |
diff --git a/.github/workflows/sanitizer.yml b/.github/workflows/sanitizer.yml
@@ -40,7 +40,7 @@ jobs:
       - name: cmake
         shell: bash
         run: |
-          cmake -DCMAKE_BUILD_TYPE=Release -DLOGGING_HIDE_ADDRESSES=ON -D CMAKE_C_COMPILER_LAUNCHER=ccache -D CMAKE_CXX_COMPILER_LAUNCHER=ccache ${{ matrix.flags }}  -G Ninja -S . -B .
+          cmake -DCMAKE_BUILD_TYPE=Release -D CMAKE_C_COMPILER_LAUNCHER=ccache -D CMAKE_CXX_COMPILER_LAUNCHER=ccache ${{ matrix.flags }}  -G Ninja -S . -B .
       - name: build
         shell: bash
         run: |
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -15,7 +15,6 @@ cmake_dependent_option(NAUTILUS_DOWNLOAD_MLIR "USE_PRE_BUILD_MLIR" ON "ENABLE_ML
 cmake_dependent_option(ENABLE_C_BACKEND "Enable the C compiler backend" ON "ENABLE_COMPILER" OFF)
 cmake_dependent_option(ENABLE_BC_BACKEND "Enable the bytecode interpreter backend" ON "ENABLE_COMPILER" OFF)
 cmake_dependent_option(ENABLE_ASMJIT_BACKEND "Enable the asmjit interpreter backend" OFF "ENABLE_COMPILER" OFF)
-option(LOGGING_HIDE_ADDRESSES "Enable logging of addresses (disable for testing)" OFF)
 
 option(ENABLE_TEST_COVERAGE "Enable the collection of test coverage." OFF)
 option(ENABLE_ADDRESS_SANITIZER "Enable address sanitizer." OFF)
diff --git a/nautilus/include/nautilus/common/config.h.in b/nautilus/include/nautilus/common/config.h.in
@@ -10,5 +10,4 @@
 #cmakedefine TEST_DATA_FOLDER "@TEST_DATA_FOLDER@"
 
 // Backends
-#cmakedefine NAUTILUS_ACTIVATE_MLIR_BACKEND
-#cmakedefine LOGGING_HIDE_ADDRESSES
+#cmakedefine NAUTILUS_ACTIVATE_MLIR_BACKEND
diff --git a/nautilus/src/nautilus/compiler/ir/IRGraph.cpp b/nautilus/src/nautilus/compiler/ir/IRGraph.cpp
@@ -17,6 +17,7 @@
 #include "nautilus/compiler/ir/operations/ProxyCallOperation.hpp"
 #include "nautilus/compiler/ir/operations/ReturnOperation.hpp"
 #include "nautilus/compiler/ir/operations/StoreOperation.hpp"
+#include "nautilus/logging.hpp"
 #include <fmt/format.h>
 #include <utility>
 
@@ -119,8 +120,8 @@ struct formatter<nautilus::compiler::ir::IRGraph> : formatter<std::string_view>
 
 template <>
 struct formatter<nautilus::compiler::ir::OperationIdentifier> : formatter<std::string_view> {
-	static auto format(const nautilus::compiler::ir::OperationIdentifier& op, format_context& ctx)
-	    -> format_context::iterator {
+	static auto format(const nautilus::compiler::ir::OperationIdentifier& op,
+	                   format_context& ctx) -> format_context::iterator {
 		auto out = ctx.out();
 		fmt::format_to(out, "${}", op.getId());
 		return out;
@@ -129,8 +130,8 @@ struct formatter<nautilus::compiler::ir::OperationIdentifier> : formatter<std::s
 
 template <>
 struct formatter<nautilus::compiler::ir::BasicBlockInvocation> : formatter<std::string_view> {
-	static auto format(const nautilus::compiler::ir::BasicBlockInvocation& op, format_context& ctx)
-	    -> format_context::iterator {
+	static auto format(const nautilus::compiler::ir::BasicBlockInvocation& op,
+	                   format_context& ctx) -> format_context::iterator {
 		auto out = ctx.out();
 		fmt::format_to(out, "Block_{}(", op.getBlock()->getIdentifier());
 		const auto& args = op.getArguments();
@@ -157,18 +158,18 @@ struct formatter<nautilus::compiler::ir::IfOperation> : formatter<std::string_vi
 
 template <>
 struct formatter<nautilus::compiler::ir::ProxyCallOperation> : formatter<std::string_view> {
-	static auto format(const nautilus::compiler::ir::ProxyCallOperation& op, format_context& ctx)
-	    -> format_context::iterator {
+	static auto format(const nautilus::compiler::ir::ProxyCallOperation& op,
+	                   format_context& ctx) -> format_context::iterator {
 		auto out = ctx.out();
 
 		if (op.getStamp() != nautilus::Type::v) {
 			fmt::format_to(out, "${} = ", op.getIdentifier().getId());
 		}
-#ifdef LOGGING_HIDE_ADDRESSES
-			fmt::format_to(out, "func_*(");
-#else
+		if (nautilus::log::options::getLogAddresses()) {
 			fmt::format_to(out, "{}(", op.getFunctionName());
-#endif
+		} else {
+			fmt::format_to(out, "func_*(");
+		}
 		const auto& args = op.getInputArguments();
 		for (size_t i = 0; i < args.size(); ++i) {
 			if (i > 0) {
@@ -238,8 +239,8 @@ struct formatter<nautilus::compiler::ir::Operation> : formatter<std::string_view
 
 template <>
 struct formatter<nautilus::compiler::ir::BasicBlock> : formatter<std::string_view> {
-	static auto format(const nautilus::compiler::ir::BasicBlock& block, format_context& ctx)
-	    -> format_context::iterator {
+	static auto format(const nautilus::compiler::ir::BasicBlock& block,
+	                   format_context& ctx) -> format_context::iterator {
 		auto out = ctx.out();
 		fmt::format_to(out, "\nBlock_{}(", block.getIdentifier());
 		const auto& args = block.getArguments();
@@ -260,8 +261,8 @@ struct formatter<nautilus::compiler::ir::BasicBlock> : formatter<std::string_vie
 
 template <>
 struct formatter<nautilus::compiler::ir::FunctionOperation> : formatter<std::string_view> {
-	static auto format(const nautilus::compiler::ir::FunctionOperation& func, format_context& ctx)
-	    -> format_context::iterator {
+	static auto format(const nautilus::compiler::ir::FunctionOperation& func,
+	                   format_context& ctx) -> format_context::iterator {
 		auto out = ctx.out();
 		fmt::format_to(out, "{}(", func.getName());
 		for (const auto& arg : func.getInputArgNames()) {
diff --git a/nautilus/src/nautilus/logging.cpp b/nautilus/src/nautilus/logging.cpp
@@ -1 +1,11 @@
 #include <nautilus/logging.hpp>
+
+namespace nautilus::log::options {
+static bool logAddresses = true;
+bool getLogAddresses() {
+	return logAddresses;
+}
+void setLogAddresses(bool value) {
+	logAddresses = value;
+}
+} // namespace nautilus::log::options
diff --git a/nautilus/src/nautilus/logging.hpp b/nautilus/src/nautilus/logging.hpp
@@ -7,7 +7,7 @@
 
 namespace nautilus::log {
 template <typename... Args>
-void info([[maybe_unused]] const char* fmt, [[maybe_unused]]Args&&... args) {
+void info([[maybe_unused]] const char* fmt, [[maybe_unused]] Args&&... args) {
 #ifdef ENABLE_LOGGING
 	spdlog::info("{}", fmt::format(fmt::runtime(fmt), std::forward<Args>(args)...));
 #endif
@@ -23,7 +23,7 @@ void debug([[maybe_unused]] const char* fmt, [[maybe_unused]] Args&&... args) {
 template <typename... Args>
 void trace([[maybe_unused]] const char* fmt, [[maybe_unused]] Args&&... args) {
 #ifdef ENABLE_LOGGING
-	spdlog::trace("{}",fmt::format(fmt::runtime(fmt), std::forward<Args>(args)...));
+	spdlog::trace("{}", fmt::format(fmt::runtime(fmt), std::forward<Args>(args)...));
 #endif
 }
 
@@ -33,4 +33,11 @@ void error([[maybe_unused]] const char* fmt, [[maybe_unused]] Args&&... args) {
 	spdlog::error("{}", fmt::format(fmt::runtime(fmt), std::forward<Args>(args)...));
 #endif
 }
+
+namespace options {
+
+bool getLogAddresses();
+void setLogAddresses(bool);
+
+} // namespace options
 } // namespace nautilus::log
diff --git a/nautilus/src/nautilus/tracing/ExecutionTrace.cpp b/nautilus/src/nautilus/tracing/ExecutionTrace.cpp
@@ -4,6 +4,7 @@
 #include <algorithm>
 #include <fmt/format.h>
 #include <nautilus/config.hpp>
+#include <nautilus/logging.hpp>
 
 namespace nautilus::tracing {
 
@@ -283,8 +284,8 @@ auto formatter<nautilus::tracing::ExecutionTrace>::format(const nautilus::tracin
 	return out;
 }
 
-auto formatter<nautilus::tracing::Block>::format(const nautilus::tracing::Block& block, format_context& ctx)
-    -> format_context::iterator {
+auto formatter<nautilus::tracing::Block>::format(const nautilus::tracing::Block& block,
+                                                 format_context& ctx) -> format_context::iterator {
 	auto out = ctx.out();
 	fmt::format_to(out, "(");
 	for (size_t i = 0; i < block.arguments.size(); i++) {
@@ -306,8 +307,8 @@ auto formatter<nautilus::tracing::Block>::format(const nautilus::tracing::Block&
 
 template <>
 struct formatter<nautilus::tracing::TypedValueRef> : formatter<std::string_view> {
-	static auto format(const nautilus::tracing::TypedValueRef& typeValRef, format_context& ctx)
-	    -> format_context::iterator {
+	static auto format(const nautilus::tracing::TypedValueRef& typeValRef,
+	                   format_context& ctx) -> format_context::iterator {
 		auto out = ctx.out();
 		fmt::format_to(out, "${}", typeValRef.ref);
 		return out;
@@ -334,11 +335,12 @@ template <>
 struct formatter<nautilus::tracing::FunctionCall> : formatter<std::string_view> {
 	static auto format(const nautilus::tracing::FunctionCall& call, format_context& ctx) -> format_context::iterator {
 		auto out = ctx.out();
-#ifdef LOGGING_HIDE_ADDRESSES
-		fmt::format_to(out, "func_*(");
-#else
-		fmt::format_to(out, "{}(", call.functionName);
-#endif
+		if (nautilus::log::options::getLogAddresses()) {
+			fmt::format_to(out, "{}(", call.functionName);
+		} else {
+			fmt::format_to(out, "func_*(");
+		}
+
 		for (size_t i = 0; i < call.arguments.size(); i++) {
 			if (i != 0) {
 				fmt::format_to(out, ",");
diff --git a/nautilus/test/execution-tests/CMakeLists.txt b/nautilus/test/execution-tests/CMakeLists.txt
@@ -14,23 +14,27 @@ list(APPEND CMAKE_MODULE_PATH ${catch2_SOURCE_DIR}/extras)
 
 catch_discover_tests(nautilus-execution-tests EXTRA_ARGS --allow-running-no-tests)
 
-if (ENABLE_TRACING AND (CMAKE_CXX_COMPILER_ID MATCHES "Clang"))
-    # using Clang
-    add_executable(nautilus-tracing-tests
-            TracingTest.cpp
-    )
-
-    target_include_directories(nautilus-tracing-tests PRIVATE
-            $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/../common>
-            $<INSTALL_INTERFACE:src/nautilus/>)
-
-    target_include_directories(nautilus-tracing-tests PRIVATE
-            $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/../../src>
-            $<INSTALL_INTERFACE:src/nautilus/>)
-
-    target_link_libraries(nautilus-tracing-tests PUBLIC nautilus Catch2::Catch2WithMain)
-    list(APPEND CMAKE_MODULE_PATH ${catch2_SOURCE_DIR}/extras)
-    catch_discover_tests(nautilus-tracing-tests EXTRA_ARGS --allow-running-no-tests)
+if (ENABLE_TRACING)
+    if (CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+        # using Clang
+        add_executable(nautilus-tracing-tests
+                TracingTest.cpp
+        )
+
+        target_include_directories(nautilus-tracing-tests PRIVATE
+                $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/../common>
+                $<INSTALL_INTERFACE:src/nautilus/>)
+
+        target_include_directories(nautilus-tracing-tests PRIVATE
+                $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/../../src>
+                $<INSTALL_INTERFACE:src/nautilus/>)
+
+        target_link_libraries(nautilus-tracing-tests PUBLIC nautilus Catch2::Catch2WithMain)
+        list(APPEND CMAKE_MODULE_PATH ${catch2_SOURCE_DIR}/extras)
+        catch_discover_tests(nautilus-tracing-tests EXTRA_ARGS --allow-running-no-tests)
+    else ()
+        message(WARNING "Tracing Tests are only available on clang-based compilers!")
+    endif ()
 endif ()
 
 add_subdirectory(std)
diff --git a/nautilus/test/execution-tests/TracingTest.cpp b/nautilus/test/execution-tests/TracingTest.cpp
@@ -20,6 +20,12 @@
 #include <iostream>
 #include <sstream>
 
+namespace nautilus::log::options {
+
+bool getLogAddresses();
+void setLogAddresses(bool);
+
+} // namespace nautilus::log::options
 namespace nautilus::engine {
 
 namespace details {
@@ -132,6 +138,8 @@ bool checkTestFile(T* trace, const std::string category, const std::string group
 }
 
 void runTraceTests(const std::string& category, std::vector<std::tuple<std::string, std::function<void()>>>& tests) {
+	// disable logging of addresses such that the trace is deterministic
+	nautilus::log::options::setLogAddresses(false);
 	for (auto& test : tests) {
 		auto func = std::get<1>(test);
 		auto name = std::get<0>(test);
