[PATCH] D22611: [compiler-rt][XRay] re-submitting r276117, with fixes for build breakage due to extraneous and missing dependencies and attempts to build on unsupported OSes
Dean Michael Berris via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 21 00:14:46 PDT 2016
dberris added inline comments.
================
Comment at: lib/CMakeLists.txt:61-68
@@ +60,9 @@
+
+if(COMPILER_RT_BUILD_XRAY)
+ if(COMPILER_RT_HAS_XRAY AND COMPILER_RT_HAS_SANITIZER_COMMON)
+ if (NOT COMPILER_RT_BUILD_SANITIZERS)
+ add_subdirectory(sanitizer_common)
+ endif()
+ add_subdirectory(xray)
+ endif()
+endif()
----------------
echristo wrote:
> This looks unnecessarily complicated?
Right. The other option was to just lump XRay into the sanitizers above. But that doesn't allow us to just build XRay -- or at least, XRay isn't strictly a sanitizer even.
If we want to allow building just XRay which depends on sanitizer_common for the flag processing then we need to guard that somehow.
This is the Nth iteration for this that I can convince myself is succinct enough to convey the intent. It's really that:
- if you want to build xray
- and you can also build sanitizer common
- if you didn't build any of the other sanitizers
- add dependency to build sanitizer_common
- build xray
Other more invasive change involves building sanitizer_common if either we build the sanitizers or we build xray.
I can go with that approach, which might make this part of the stuff less complicated.
WDYT?
https://reviews.llvm.org/D22611
More information about the llvm-commits
mailing list