[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