[PATCH] D38306: Cleanup some problems with LLVM_ENABLE_DUMP in release builds.

[Don Hinton via Phabricator via llvm-commits]

hintonda added inline comments.


================
Comment at: CMakeLists.txt:397-401
+if( LLVM_ENABLE_ASSERTIONS )
+	option(LLVM_ENABLE_DUMP "Enable dump functions in release builds" ON)
+else()
+	option(LLVM_ENABLE_DUMP "Enable dump functions in release builds" OFF)
+endif()
----------------
MatzeB wrote:
> This is fragile, as for any subsequent `cmake` runs the value for LLVM_ENABLE_DUMP is already set in the cmake cache and won't change even if the user modifies `LLVM_ENABLE_ASSERTIONS`.
> 
> We rather want cmake code that sets the define if either this option or LLVM_ENABLE_ASSERTIONS is enabled.
So, are you saying it should always be set for +Asserts builds?

The email thread seemed to indicate they should vary independently.


================
Comment at: lib/CodeGen/ScoreboardHazardRecognizer.cpp:35
       DAG(SchedDAG) {
+  (void)DebugType;
   // Determine the maximum depth of any itinerary. This determines the depth of
----------------
MatzeB wrote:
> Is this to silence a warning about that member being unused? Maybe we better put an #ifdef around the member then?
Yes, and I considered that, but you'd also have to #ifdef away the ctor initializer which seems really ugly to me.  Please let me know which you prefer.


https://reviews.llvm.org/D38306