[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)
Vy Nguyen via lldb-commits
lldb-commits at lists.llvm.org
Wed Feb 26 07:13:41 PST 2025
https://github.com/oontvoo updated https://github.com/llvm/llvm-project/pull/128534
>From 236ab76b7c9433f8fed6586a4ee18351779cc191 Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Mon, 24 Feb 2025 11:32:03 -0500
Subject: [PATCH 01/13] [llvm][telemetr]Change Telemetry-disabling mechanism.
Details:
- Previously, we used the LLVM_BUILD_TELEMETRY flag to control whether any Telemetry code will be built.
This has proven to cause more nuisance to both users of the Telemetry and any further extension of it.
(Eg., we needed to put #ifdef around caller/user code)
- So the new approach is to:
+ Remove this flag and introduce LLVM_ENABLE_TELEMETRY which would be true by default.
+ If LLVM_ENABLE_TELEMETRY is set to FALSE (at buildtime), the library would still be built BUT Telemetry cannot be enabled.
And no data can be collected.
The benefit of this is that it simplifies user (and extension) code since we just need to put the check on Config::EnableTelemetry.
Besides, the Telemetry library itself is very small, hence the additional code to be built would not cause any difference in build performance.
---
lldb/source/Core/CMakeLists.txt | 4 +---
lldb/source/Core/Telemetry.cpp | 7 ++-----
lldb/unittests/Core/CMakeLists.txt | 6 +++---
llvm/CMakeLists.txt | 3 ++-
llvm/cmake/modules/LLVMConfig.cmake.in | 1 +
llvm/include/llvm/Config/llvm-config.h.cmake | 2 ++
llvm/include/llvm/Telemetry/Telemetry.h | 12 ++++++++++--
llvm/lib/CMakeLists.txt | 5 ++---
llvm/unittests/CMakeLists.txt | 4 +---
.../gn/secondary/llvm/include/llvm/Config/BUILD.gn | 1 +
utils/bazel/llvm_configs/llvm-config.h.cmake | 2 ++
11 files changed, 27 insertions(+), 20 deletions(-)
diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt
index 82fb5f42f9f4b..a3c12e4c1bdbc 100644
--- a/lldb/source/Core/CMakeLists.txt
+++ b/lldb/source/Core/CMakeLists.txt
@@ -16,9 +16,7 @@ if (LLDB_ENABLE_CURSES)
endif()
endif()
-if (LLVM_BUILD_TELEMETRY)
- set(TELEMETRY_DEPS Telemetry)
-endif()
+set(TELEMETRY_DEPS Telemetry)
# TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbCore
add_lldb_library(lldbCore
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 5222f76704f91..51a860ea5313b 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -8,8 +8,6 @@
#include "llvm/Config/llvm-config.h"
-#ifdef LLVM_BUILD_TELEMETRY
-
#include "lldb/Core/Telemetry.h"
#include "lldb/Core/Debugger.h"
#include "lldb/Utility/LLDBLog.h"
@@ -57,7 +55,8 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const {
}
TelemetryManager::TelemetryManager(std::unique_ptr<Config> config)
- : m_config(std::move(config)) {}
+ : m_config(std::move(config))
+}
llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
// Do nothing for now.
@@ -75,5 +74,3 @@ void TelemetryManager::setInstance(std::unique_ptr<TelemetryManager> manager) {
} // namespace telemetry
} // namespace lldb_private
-
-#endif // LLVM_BUILD_TELEMETRY
diff --git a/lldb/unittests/Core/CMakeLists.txt b/lldb/unittests/Core/CMakeLists.txt
index d4d3764b67ae3..e8df299631e2e 100644
--- a/lldb/unittests/Core/CMakeLists.txt
+++ b/lldb/unittests/Core/CMakeLists.txt
@@ -1,6 +1,6 @@
-if (LLVM_BUILD_TELEMETRY)
- set(TELEMETRY_DEPS Telemetry)
-endif()
+
+set(TELEMETRY_DEPS Telemetry)
+
add_lldb_unittest(LLDBCoreTests
CommunicationTest.cpp
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 88512d0f1dd96..9188fb93b7846 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -835,7 +835,8 @@ option (LLVM_ENABLE_DOXYGEN "Use doxygen to generate llvm API documentation." OF
option (LLVM_ENABLE_SPHINX "Use Sphinx to generate llvm documentation." OFF)
option (LLVM_ENABLE_OCAMLDOC "Build OCaml bindings documentation." ON)
option (LLVM_ENABLE_BINDINGS "Build bindings." ON)
-option (LLVM_BUILD_TELEMETRY "Build the telemetry library. This does not enable telemetry." ON)
+option (LLVM_BUILD_TELEMETRY "[DEPRECATED - use LLVM_ENABLE_TELEMTRY]Enable the telemetry library. If set to OFF, library cannot be enabled after build (eg., at runtime)" ON)
+option (LLVM_ENABLE_TELEMETRY "Enable the telemetry library. If set to OFF, library cannot be enabled after build (eg., at runtime)" ON)
set(LLVM_INSTALL_DOXYGEN_HTML_DIR "${CMAKE_INSTALL_DOCDIR}/llvm/doxygen-html"
CACHE STRING "Doxygen-generated HTML documentation install directory")
diff --git a/llvm/cmake/modules/LLVMConfig.cmake.in b/llvm/cmake/modules/LLVMConfig.cmake.in
index 28655ee3ab87d..134d107ce79ba 100644
--- a/llvm/cmake/modules/LLVMConfig.cmake.in
+++ b/llvm/cmake/modules/LLVMConfig.cmake.in
@@ -101,6 +101,7 @@ set(LLVM_ENABLE_PIC @LLVM_ENABLE_PIC@)
set(LLVM_BUILD_32_BITS @LLVM_BUILD_32_BITS@)
set(LLVM_BUILD_TELEMETRY @LLVM_BUILD_TELEMETRY@)
+set(LLVM_EANBLE_TELEMETRY @LLVM_ENABLE_TELEMETRY@)
if (NOT "@LLVM_PTHREAD_LIB@" STREQUAL "")
set(LLVM_PTHREAD_LIB "@LLVM_PTHREAD_LIB@")
diff --git a/llvm/include/llvm/Config/llvm-config.h.cmake b/llvm/include/llvm/Config/llvm-config.h.cmake
index 239f9dd3f38db..13db9935870f8 100644
--- a/llvm/include/llvm/Config/llvm-config.h.cmake
+++ b/llvm/include/llvm/Config/llvm-config.h.cmake
@@ -202,6 +202,8 @@
#cmakedefine LLVM_HAS_LOGF128
/* Define if building LLVM with LLVM_BUILD_TELEMETRY */
+/* DEPRECATED - use LLVM_ENABLE_TELEMETRY */
#cmakedefine LLVM_BUILD_TELEMETRY ${LLVM_BUILD_TELEMETRY}
+#cmakedefine LLVM_ENABLE_TELEMETRY ${LLVM_ENABLE_TELEMETRY}
#endif
diff --git a/llvm/include/llvm/Telemetry/Telemetry.h b/llvm/include/llvm/Telemetry/Telemetry.h
index 42319f3ef51f2..f60c4c762ca7b 100644
--- a/llvm/include/llvm/Telemetry/Telemetry.h
+++ b/llvm/include/llvm/Telemetry/Telemetry.h
@@ -64,11 +64,19 @@ class Serializer {
/// This struct can be extended as needed to add additional configuration
/// points specific to a vendor's implementation.
struct Config {
- virtual ~Config() = default;
+#ifdef LLVM_ENABLE_TELEMETRY
+ static bool BuildTimeEnableTelemetry = true;
+#else
+ static bool BuildTimeEnableTelemetry = false;
+#endif
+ virtual ~Config() : EnableTelemetry(BuildTimeEnableTelemetry) {}
// If true, telemetry will be enabled.
const bool EnableTelemetry;
- Config(bool E) : EnableTelemetry(E) {}
+
+ // Telemetry can only be enabled if both the runtime and buildtime flag
+ // are set.
+ Config(bool E) : EnableTelemetry(E && BuildTimeEnableTelemetry) {}
virtual std::optional<std::string> makeSessionId() { return std::nullopt; }
};
diff --git a/llvm/lib/CMakeLists.txt b/llvm/lib/CMakeLists.txt
index d0a2bc9294381..37b6fcf1236e3 100644
--- a/llvm/lib/CMakeLists.txt
+++ b/llvm/lib/CMakeLists.txt
@@ -41,9 +41,8 @@ add_subdirectory(ProfileData)
add_subdirectory(Passes)
add_subdirectory(TargetParser)
add_subdirectory(TextAPI)
-if (LLVM_BUILD_TELEMETRY)
- add_subdirectory(Telemetry)
-endif()
+add_subdirectory(Telemetry)
+
add_subdirectory(ToolDrivers)
add_subdirectory(XRay)
if (LLVM_INCLUDE_TESTS)
diff --git a/llvm/unittests/CMakeLists.txt b/llvm/unittests/CMakeLists.txt
index 12e229b1c3498..81abce51b8939 100644
--- a/llvm/unittests/CMakeLists.txt
+++ b/llvm/unittests/CMakeLists.txt
@@ -63,9 +63,7 @@ add_subdirectory(Support)
add_subdirectory(TableGen)
add_subdirectory(Target)
add_subdirectory(TargetParser)
-if (LLVM_BUILD_TELEMETRY)
- add_subdirectory(Telemetry)
-endif()
+add_subdirectory(Telemetry)
add_subdirectory(Testing)
add_subdirectory(TextAPI)
add_subdirectory(Transforms)
diff --git a/llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn b/llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
index 5a13545a15b13..8f6c431a713af 100644
--- a/llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
+++ b/llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
@@ -293,6 +293,7 @@ write_cmake_config("llvm-config") {
"LLVM_BUILD_LLVM_DYLIB=",
"LLVM_BUILD_SHARED_LIBS=",
"LLVM_BUILD_TELEMETRY=",
+ "LLVM_ENABLE_TELEMETRY=",
"LLVM_DEFAULT_TARGET_TRIPLE=$llvm_target_triple",
"LLVM_ENABLE_DUMP=",
"LLVM_ENABLE_HTTPLIB=",
diff --git a/utils/bazel/llvm_configs/llvm-config.h.cmake b/utils/bazel/llvm_configs/llvm-config.h.cmake
index 239f9dd3f38db..315ddfb37362c 100644
--- a/utils/bazel/llvm_configs/llvm-config.h.cmake
+++ b/utils/bazel/llvm_configs/llvm-config.h.cmake
@@ -202,6 +202,8 @@
#cmakedefine LLVM_HAS_LOGF128
/* Define if building LLVM with LLVM_BUILD_TELEMETRY */
+/* DEPRECATED - Use LLVM_ENABLE_TELEMETRY */
#cmakedefine LLVM_BUILD_TELEMETRY ${LLVM_BUILD_TELEMETRY}
+#cmakedefine LLVM_ENABLE_TELEMETRY ${LLVM_ENABLE_TELEMETRY}
#endif
>From 45869d7a8e18bd2334c2aab7e62adddffaf93d1d Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Mon, 24 Feb 2025 13:01:44 -0500
Subject: [PATCH 02/13] formatting + more checks
---
lldb/source/Core/Telemetry.cpp | 9 ++++++---
llvm/include/llvm/Telemetry/Telemetry.h | 4 ++--
llvm/lib/Telemetry/Telemetry.cpp | 2 ++
3 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 51a860ea5313b..865488928d25d 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -55,8 +55,7 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const {
}
TelemetryManager::TelemetryManager(std::unique_ptr<Config> config)
- : m_config(std::move(config))
-}
+ : m_config(std::move(config)) {}
llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
// Do nothing for now.
@@ -66,7 +65,11 @@ llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
}
std::unique_ptr<TelemetryManager> TelemetryManager::g_instance = nullptr;
-TelemetryManager *TelemetryManager::getInstance() { return g_instance.get(); }
+TelemetryManager *TelemetryManager::getInstance() {
+ if (!Config::BuildTimeEnableTelemetry)
+ return nullptr;
+ return g_instance.get();
+}
void TelemetryManager::setInstance(std::unique_ptr<TelemetryManager> manager) {
g_instance = std::move(manager);
diff --git a/llvm/include/llvm/Telemetry/Telemetry.h b/llvm/include/llvm/Telemetry/Telemetry.h
index f60c4c762ca7b..bed3eca6fde5e 100644
--- a/llvm/include/llvm/Telemetry/Telemetry.h
+++ b/llvm/include/llvm/Telemetry/Telemetry.h
@@ -65,9 +65,9 @@ class Serializer {
/// points specific to a vendor's implementation.
struct Config {
#ifdef LLVM_ENABLE_TELEMETRY
- static bool BuildTimeEnableTelemetry = true;
+ static const bool BuildTimeEnableTelemetry = true;
#else
- static bool BuildTimeEnableTelemetry = false;
+ static const bool BuildTimeEnableTelemetry = false;
#endif
virtual ~Config() : EnableTelemetry(BuildTimeEnableTelemetry) {}
diff --git a/llvm/lib/Telemetry/Telemetry.cpp b/llvm/lib/Telemetry/Telemetry.cpp
index d86ad9c1c37bb..caac7e6c332cb 100644
--- a/llvm/lib/Telemetry/Telemetry.cpp
+++ b/llvm/lib/Telemetry/Telemetry.cpp
@@ -21,6 +21,8 @@ void TelemetryInfo::serialize(Serializer &serializer) const {
}
Error Manager::dispatch(TelemetryInfo *Entry) {
+ assert(Config::BuildTimeEnableTelemetry &&
+ "Telemetry should have been enabled");
if (Error Err = preDispatch(Entry))
return Err;
>From b199854ac2e7100eac7fea20e9464dcb9938122c Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Mon, 24 Feb 2025 13:10:21 -0500
Subject: [PATCH 03/13] more clean up
---
lldb/source/Core/CMakeLists.txt | 4 +---
lldb/source/Core/Telemetry.cpp | 3 ---
lldb/unittests/Core/CMakeLists.txt | 5 +----
lldb/unittests/Core/TelemetryTest.cpp | 7 -------
llvm/CMakeLists.txt | 2 +-
llvm/include/llvm/Config/llvm-config.h.cmake | 2 ++
utils/bazel/llvm_configs/llvm-config.h.cmake | 2 ++
7 files changed, 7 insertions(+), 18 deletions(-)
diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt
index a3c12e4c1bdbc..d5d8a9d5088fc 100644
--- a/lldb/source/Core/CMakeLists.txt
+++ b/lldb/source/Core/CMakeLists.txt
@@ -16,8 +16,6 @@ if (LLDB_ENABLE_CURSES)
endif()
endif()
-set(TELEMETRY_DEPS Telemetry)
-
# TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbCore
add_lldb_library(lldbCore
Address.cpp
@@ -82,7 +80,7 @@ add_lldb_library(lldbCore
Support
Demangle
TargetParser
- ${TELEMETRY_DEPS}
+ Telemetry
)
add_dependencies(lldbCore
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 865488928d25d..428bdbbdfb602 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -5,9 +5,6 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
-
-#include "llvm/Config/llvm-config.h"
-
#include "lldb/Core/Telemetry.h"
#include "lldb/Core/Debugger.h"
#include "lldb/Utility/LLDBLog.h"
diff --git a/lldb/unittests/Core/CMakeLists.txt b/lldb/unittests/Core/CMakeLists.txt
index e8df299631e2e..60265f794b5e8 100644
--- a/lldb/unittests/Core/CMakeLists.txt
+++ b/lldb/unittests/Core/CMakeLists.txt
@@ -1,7 +1,4 @@
-set(TELEMETRY_DEPS Telemetry)
-
-
add_lldb_unittest(LLDBCoreTests
CommunicationTest.cpp
DiagnosticEventTest.cpp
@@ -31,5 +28,5 @@ add_lldb_unittest(LLDBCoreTests
LLVMTestingSupport
LINK_COMPONENTS
Support
- ${TELEMETRY_DEPS}
+ Telemetry
)
diff --git a/lldb/unittests/Core/TelemetryTest.cpp b/lldb/unittests/Core/TelemetryTest.cpp
index 0f2eaccb21a2c..caae31a42641f 100644
--- a/lldb/unittests/Core/TelemetryTest.cpp
+++ b/lldb/unittests/Core/TelemetryTest.cpp
@@ -5,11 +5,6 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
-
-#include "llvm/Config/llvm-config.h"
-
-#ifdef LLVM_BUILD_TELEMETRY
-
#include "lldb/Core/PluginInterface.h"
#include "lldb/Core/PluginManager.h"
#include "lldb/Core/Telemetry.h"
@@ -94,5 +89,3 @@ TEST(TelemetryTest, PluginTest) {
ASSERT_EQ("FakeTelemetryPlugin", ins->GetInstanceName());
}
-
-#endif // LLVM_BUILD_TELEMETRY
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 9188fb93b7846..30864beb29e7d 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -835,7 +835,7 @@ option (LLVM_ENABLE_DOXYGEN "Use doxygen to generate llvm API documentation." OF
option (LLVM_ENABLE_SPHINX "Use Sphinx to generate llvm documentation." OFF)
option (LLVM_ENABLE_OCAMLDOC "Build OCaml bindings documentation." ON)
option (LLVM_ENABLE_BINDINGS "Build bindings." ON)
-option (LLVM_BUILD_TELEMETRY "[DEPRECATED - use LLVM_ENABLE_TELEMTRY]Enable the telemetry library. If set to OFF, library cannot be enabled after build (eg., at runtime)" ON)
+option (LLVM_BUILD_TELEMETRY "[DEPRECATED - use LLVM_ENABLE_TELEMTRY]" ON)
option (LLVM_ENABLE_TELEMETRY "Enable the telemetry library. If set to OFF, library cannot be enabled after build (eg., at runtime)" ON)
set(LLVM_INSTALL_DOXYGEN_HTML_DIR "${CMAKE_INSTALL_DOCDIR}/llvm/doxygen-html"
diff --git a/llvm/include/llvm/Config/llvm-config.h.cmake b/llvm/include/llvm/Config/llvm-config.h.cmake
index 13db9935870f8..b184ef9cf26af 100644
--- a/llvm/include/llvm/Config/llvm-config.h.cmake
+++ b/llvm/include/llvm/Config/llvm-config.h.cmake
@@ -204,6 +204,8 @@
/* Define if building LLVM with LLVM_BUILD_TELEMETRY */
/* DEPRECATED - use LLVM_ENABLE_TELEMETRY */
#cmakedefine LLVM_BUILD_TELEMETRY ${LLVM_BUILD_TELEMETRY}
+
+/* Define if building LLVM with LLVM_ENABLE_TELEMETRY */
#cmakedefine LLVM_ENABLE_TELEMETRY ${LLVM_ENABLE_TELEMETRY}
#endif
diff --git a/utils/bazel/llvm_configs/llvm-config.h.cmake b/utils/bazel/llvm_configs/llvm-config.h.cmake
index 315ddfb37362c..30bbf47b3d342 100644
--- a/utils/bazel/llvm_configs/llvm-config.h.cmake
+++ b/utils/bazel/llvm_configs/llvm-config.h.cmake
@@ -204,6 +204,8 @@
/* Define if building LLVM with LLVM_BUILD_TELEMETRY */
/* DEPRECATED - Use LLVM_ENABLE_TELEMETRY */
#cmakedefine LLVM_BUILD_TELEMETRY ${LLVM_BUILD_TELEMETRY}
+
+/* Define if building LLVM with LLVM_ENABLE_TELEMETRY */
#cmakedefine LLVM_ENABLE_TELEMETRY ${LLVM_ENABLE_TELEMETRY}
#endif
>From 9f5cfcd7ea2ca3cc80cc21c274ddde284303d6c3 Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Tue, 25 Feb 2025 10:04:00 -0500
Subject: [PATCH 04/13] Update llvm/include/llvm/Telemetry/Telemetry.h
Co-authored-by: Pavel Labath <pavel at labath.sk>
---
llvm/include/llvm/Telemetry/Telemetry.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/include/llvm/Telemetry/Telemetry.h b/llvm/include/llvm/Telemetry/Telemetry.h
index bed3eca6fde5e..118adcb594838 100644
--- a/llvm/include/llvm/Telemetry/Telemetry.h
+++ b/llvm/include/llvm/Telemetry/Telemetry.h
@@ -76,7 +76,7 @@ struct Config {
// Telemetry can only be enabled if both the runtime and buildtime flag
// are set.
- Config(bool E) : EnableTelemetry(E && BuildTimeEnableTelemetry) {}
+ explicit Config(bool E) : EnableTelemetry(E && BuildTimeEnableTelemetry) {}
virtual std::optional<std::string> makeSessionId() { return std::nullopt; }
};
>From 361a5641704a8116f747115468e3a1aea4c6bbbc Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Tue, 25 Feb 2025 10:05:40 -0500
Subject: [PATCH 05/13] Update llvm/include/llvm/Config/llvm-config.h.cmake
Co-authored-by: Pavel Labath <pavel at labath.sk>
---
llvm/include/llvm/Config/llvm-config.h.cmake | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/include/llvm/Config/llvm-config.h.cmake b/llvm/include/llvm/Config/llvm-config.h.cmake
index b184ef9cf26af..904b49fce1db3 100644
--- a/llvm/include/llvm/Config/llvm-config.h.cmake
+++ b/llvm/include/llvm/Config/llvm-config.h.cmake
@@ -206,6 +206,6 @@
#cmakedefine LLVM_BUILD_TELEMETRY ${LLVM_BUILD_TELEMETRY}
/* Define if building LLVM with LLVM_ENABLE_TELEMETRY */
-#cmakedefine LLVM_ENABLE_TELEMETRY ${LLVM_ENABLE_TELEMETRY}
+#cmakedefine01 LLVM_ENABLE_TELEMETRY ${LLVM_ENABLE_TELEMETRY}
#endif
>From 013637aeb1a9610a3c02ed2e98ba0dad69bbd763 Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Tue, 25 Feb 2025 10:20:58 -0500
Subject: [PATCH 06/13] Update llvm/include/llvm/Telemetry/Telemetry.h
Co-authored-by: Pavel Labath <pavel at labath.sk>
---
llvm/include/llvm/Telemetry/Telemetry.h | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/llvm/include/llvm/Telemetry/Telemetry.h b/llvm/include/llvm/Telemetry/Telemetry.h
index 118adcb594838..a6938ce3db710 100644
--- a/llvm/include/llvm/Telemetry/Telemetry.h
+++ b/llvm/include/llvm/Telemetry/Telemetry.h
@@ -64,11 +64,7 @@ class Serializer {
/// This struct can be extended as needed to add additional configuration
/// points specific to a vendor's implementation.
struct Config {
-#ifdef LLVM_ENABLE_TELEMETRY
- static const bool BuildTimeEnableTelemetry = true;
-#else
- static const bool BuildTimeEnableTelemetry = false;
-#endif
+ static const bool BuildTimeEnableTelemetry = LLVM_ENABLE_TELEMETRY;
virtual ~Config() : EnableTelemetry(BuildTimeEnableTelemetry) {}
// If true, telemetry will be enabled.
>From c0a8d47aaa405029628778864ea2212c0c5529f1 Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Tue, 25 Feb 2025 10:49:27 -0500
Subject: [PATCH 07/13] make field constexpr
---
llvm/include/llvm/Telemetry/Telemetry.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/llvm/include/llvm/Telemetry/Telemetry.h b/llvm/include/llvm/Telemetry/Telemetry.h
index a6938ce3db710..93c460bf63568 100644
--- a/llvm/include/llvm/Telemetry/Telemetry.h
+++ b/llvm/include/llvm/Telemetry/Telemetry.h
@@ -64,12 +64,13 @@ class Serializer {
/// This struct can be extended as needed to add additional configuration
/// points specific to a vendor's implementation.
struct Config {
- static const bool BuildTimeEnableTelemetry = LLVM_ENABLE_TELEMETRY;
- virtual ~Config() : EnableTelemetry(BuildTimeEnableTelemetry) {}
+ static constexpr bool BuildTimeEnableTelemetry = LLVM_ENABLE_TELEMETRY;
// If true, telemetry will be enabled.
const bool EnableTelemetry;
+ virtual Config() : EnableTelemetry(BuildTimeEnableTelemetry) {}
+
// Telemetry can only be enabled if both the runtime and buildtime flag
// are set.
explicit Config(bool E) : EnableTelemetry(E && BuildTimeEnableTelemetry) {}
>From 19badf748da42a50cd0eae3b4a1454063e7b218d Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Tue, 25 Feb 2025 11:08:38 -0500
Subject: [PATCH 08/13] removed deprecated flag directly
---
llvm/cmake/modules/LLVMConfig.cmake.in | 1 -
llvm/include/llvm/Config/llvm-config.h.cmake | 4 ----
llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn | 1 -
utils/bazel/llvm_configs/llvm-config.h.cmake | 4 ----
4 files changed, 10 deletions(-)
diff --git a/llvm/cmake/modules/LLVMConfig.cmake.in b/llvm/cmake/modules/LLVMConfig.cmake.in
index 134d107ce79ba..9d292a3d07fc0 100644
--- a/llvm/cmake/modules/LLVMConfig.cmake.in
+++ b/llvm/cmake/modules/LLVMConfig.cmake.in
@@ -100,7 +100,6 @@ set(LLVM_ENABLE_PIC @LLVM_ENABLE_PIC@)
set(LLVM_BUILD_32_BITS @LLVM_BUILD_32_BITS@)
-set(LLVM_BUILD_TELEMETRY @LLVM_BUILD_TELEMETRY@)
set(LLVM_EANBLE_TELEMETRY @LLVM_ENABLE_TELEMETRY@)
if (NOT "@LLVM_PTHREAD_LIB@" STREQUAL "")
diff --git a/llvm/include/llvm/Config/llvm-config.h.cmake b/llvm/include/llvm/Config/llvm-config.h.cmake
index 904b49fce1db3..1cb138adfce36 100644
--- a/llvm/include/llvm/Config/llvm-config.h.cmake
+++ b/llvm/include/llvm/Config/llvm-config.h.cmake
@@ -201,10 +201,6 @@
/* Define if logf128 is available */
#cmakedefine LLVM_HAS_LOGF128
-/* Define if building LLVM with LLVM_BUILD_TELEMETRY */
-/* DEPRECATED - use LLVM_ENABLE_TELEMETRY */
-#cmakedefine LLVM_BUILD_TELEMETRY ${LLVM_BUILD_TELEMETRY}
-
/* Define if building LLVM with LLVM_ENABLE_TELEMETRY */
#cmakedefine01 LLVM_ENABLE_TELEMETRY ${LLVM_ENABLE_TELEMETRY}
diff --git a/llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn b/llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
index 8f6c431a713af..4a7cbe4af9e9b 100644
--- a/llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
+++ b/llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
@@ -292,7 +292,6 @@ write_cmake_config("llvm-config") {
values = [
"LLVM_BUILD_LLVM_DYLIB=",
"LLVM_BUILD_SHARED_LIBS=",
- "LLVM_BUILD_TELEMETRY=",
"LLVM_ENABLE_TELEMETRY=",
"LLVM_DEFAULT_TARGET_TRIPLE=$llvm_target_triple",
"LLVM_ENABLE_DUMP=",
diff --git a/utils/bazel/llvm_configs/llvm-config.h.cmake b/utils/bazel/llvm_configs/llvm-config.h.cmake
index 30bbf47b3d342..4fb472115a51d 100644
--- a/utils/bazel/llvm_configs/llvm-config.h.cmake
+++ b/utils/bazel/llvm_configs/llvm-config.h.cmake
@@ -201,10 +201,6 @@
/* Define if logf128 is available */
#cmakedefine LLVM_HAS_LOGF128
-/* Define if building LLVM with LLVM_BUILD_TELEMETRY */
-/* DEPRECATED - Use LLVM_ENABLE_TELEMETRY */
-#cmakedefine LLVM_BUILD_TELEMETRY ${LLVM_BUILD_TELEMETRY}
-
/* Define if building LLVM with LLVM_ENABLE_TELEMETRY */
#cmakedefine LLVM_ENABLE_TELEMETRY ${LLVM_ENABLE_TELEMETRY}
>From e6fa13800d01388955a75fd2f12cdb6913df7f7b Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Tue, 25 Feb 2025 11:13:15 -0500
Subject: [PATCH 09/13] skip test if disabled
---
llvm/unittests/Telemetry/TelemetryTest.cpp | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/llvm/unittests/Telemetry/TelemetryTest.cpp b/llvm/unittests/Telemetry/TelemetryTest.cpp
index 6b37e89f16435..2dad2b9e5197d 100644
--- a/llvm/unittests/Telemetry/TelemetryTest.cpp
+++ b/llvm/unittests/Telemetry/TelemetryTest.cpp
@@ -213,6 +213,8 @@ std::shared_ptr<Config> getTelemetryConfig(const TestContext &Ctxt) {
}
TEST(TelemetryTest, TelemetryDisabled) {
+ if (!Config::BuildTimeEnableTelemetry)
+ GTEST_SKIP();
TestContext Context;
Context.HasVendorPlugin = false;
@@ -222,6 +224,8 @@ TEST(TelemetryTest, TelemetryDisabled) {
}
TEST(TelemetryTest, TelemetryEnabled) {
+ if (!Config::BuildTimeEnableTelemetry)
+ GTEST_SKIP();
const std::string ToolName = "TelemetryTestTool";
// Preset some params.
>From 36ffad75274e1019d84db8d32bbd222264288aae Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Tue, 25 Feb 2025 15:11:00 -0500
Subject: [PATCH 10/13] fix build failures
---
llvm/include/llvm/Config/llvm-config.h.cmake | 2 +-
llvm/include/llvm/Telemetry/Telemetry.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/include/llvm/Config/llvm-config.h.cmake b/llvm/include/llvm/Config/llvm-config.h.cmake
index 1cb138adfce36..4230eb850fa4b 100644
--- a/llvm/include/llvm/Config/llvm-config.h.cmake
+++ b/llvm/include/llvm/Config/llvm-config.h.cmake
@@ -202,6 +202,6 @@
#cmakedefine LLVM_HAS_LOGF128
/* Define if building LLVM with LLVM_ENABLE_TELEMETRY */
-#cmakedefine01 LLVM_ENABLE_TELEMETRY ${LLVM_ENABLE_TELEMETRY}
+#cmakedefine01 LLVM_ENABLE_TELEMETRY
#endif
diff --git a/llvm/include/llvm/Telemetry/Telemetry.h b/llvm/include/llvm/Telemetry/Telemetry.h
index 93c460bf63568..f8d0fefd8b6a2 100644
--- a/llvm/include/llvm/Telemetry/Telemetry.h
+++ b/llvm/include/llvm/Telemetry/Telemetry.h
@@ -69,7 +69,7 @@ struct Config {
// If true, telemetry will be enabled.
const bool EnableTelemetry;
- virtual Config() : EnableTelemetry(BuildTimeEnableTelemetry) {}
+ explicit Config() : EnableTelemetry(BuildTimeEnableTelemetry) {}
// Telemetry can only be enabled if both the runtime and buildtime flag
// are set.
>From c02bb0ccd825e21fc24ef35ed82b14699604fbdd Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Wed, 26 Feb 2025 10:00:40 -0500
Subject: [PATCH 11/13] Update utils/bazel/llvm_configs/llvm-config.h.cmake
Co-authored-by: Pavel Labath <pavel at labath.sk>
---
utils/bazel/llvm_configs/llvm-config.h.cmake | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/utils/bazel/llvm_configs/llvm-config.h.cmake b/utils/bazel/llvm_configs/llvm-config.h.cmake
index 4fb472115a51d..4230eb850fa4b 100644
--- a/utils/bazel/llvm_configs/llvm-config.h.cmake
+++ b/utils/bazel/llvm_configs/llvm-config.h.cmake
@@ -202,6 +202,6 @@
#cmakedefine LLVM_HAS_LOGF128
/* Define if building LLVM with LLVM_ENABLE_TELEMETRY */
-#cmakedefine LLVM_ENABLE_TELEMETRY ${LLVM_ENABLE_TELEMETRY}
+#cmakedefine01 LLVM_ENABLE_TELEMETRY
#endif
>From e21a8b1088ee5b3dbb048a5486fd01a284fafb52 Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Wed, 26 Feb 2025 10:01:21 -0500
Subject: [PATCH 12/13] Update llvm/cmake/modules/LLVMConfig.cmake.in
Co-authored-by: Pavel Labath <pavel at labath.sk>
---
llvm/cmake/modules/LLVMConfig.cmake.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/cmake/modules/LLVMConfig.cmake.in b/llvm/cmake/modules/LLVMConfig.cmake.in
index 9d292a3d07fc0..5ccc66b8039bf 100644
--- a/llvm/cmake/modules/LLVMConfig.cmake.in
+++ b/llvm/cmake/modules/LLVMConfig.cmake.in
@@ -100,7 +100,7 @@ set(LLVM_ENABLE_PIC @LLVM_ENABLE_PIC@)
set(LLVM_BUILD_32_BITS @LLVM_BUILD_32_BITS@)
-set(LLVM_EANBLE_TELEMETRY @LLVM_ENABLE_TELEMETRY@)
+set(LLVM_ENABLE_TELEMETRY @LLVM_ENABLE_TELEMETRY@)
if (NOT "@LLVM_PTHREAD_LIB@" STREQUAL "")
set(LLVM_PTHREAD_LIB "@LLVM_PTHREAD_LIB@")
>From c26546763a9fbf8e467a535f543c32d774fea8f6 Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Wed, 26 Feb 2025 10:10:21 -0500
Subject: [PATCH 13/13] define test-macro helper to skip tests when telemetry
is disabled
---
lldb/unittests/Core/TelemetryTest.cpp | 9 ++++++++-
llvm/unittests/Telemetry/TelemetryTest.cpp | 15 +++++++++------
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/lldb/unittests/Core/TelemetryTest.cpp b/lldb/unittests/Core/TelemetryTest.cpp
index 11a2839647a76..5bb46c65ace3d 100644
--- a/lldb/unittests/Core/TelemetryTest.cpp
+++ b/lldb/unittests/Core/TelemetryTest.cpp
@@ -66,7 +66,14 @@ class FakePlugin : public telemetry::TelemetryManager {
} // namespace lldb_private
-TEST(TelemetryTest, PluginTest) {
+#if LLVM_ENABLE_TELEMETRY
+#define TELEMETRY_TEST(suite, test) TEST(suite, test)
+#else
+#define TELEMETRY_TEST(suite, test) TEST(DISABLED_##suite, test)
+#endif
+
+
+TELEMETRY_TEST(TelemetryTest, PluginTest) {
// This would have been called by the plugin reg in a "real" plugin
// For tests, we just call it directly.
lldb_private::FakePlugin::Initialize();
diff --git a/llvm/unittests/Telemetry/TelemetryTest.cpp b/llvm/unittests/Telemetry/TelemetryTest.cpp
index 2dad2b9e5197d..b96ec0503d77f 100644
--- a/llvm/unittests/Telemetry/TelemetryTest.cpp
+++ b/llvm/unittests/Telemetry/TelemetryTest.cpp
@@ -212,9 +212,14 @@ std::shared_ptr<Config> getTelemetryConfig(const TestContext &Ctxt) {
return std::make_shared<Config>(false);
}
-TEST(TelemetryTest, TelemetryDisabled) {
- if (!Config::BuildTimeEnableTelemetry)
- GTEST_SKIP();
+#if LLVM_ENABLE_TELEMETRY
+#define TELEMETRY_TEST(suite, test) TEST(suite, test)
+#else
+#define TELEMETRY_TEST(suite, test) TEST(DISABLED_##suite, test)
+#endif
+
+
+TELEMETRY_TEST(TelemetryTest, TelemetryDisabled) {
TestContext Context;
Context.HasVendorPlugin = false;
@@ -223,9 +228,7 @@ TEST(TelemetryTest, TelemetryDisabled) {
EXPECT_EQ(nullptr, Manager);
}
-TEST(TelemetryTest, TelemetryEnabled) {
- if (!Config::BuildTimeEnableTelemetry)
- GTEST_SKIP();
+TELEMETRY_TEST(TelemetryTest, TelemetryEnabled) {
const std::string ToolName = "TelemetryTestTool";
// Preset some params.
More information about the lldb-commits
mailing list