Keep NDEBUG out of header in order to build llvm with cmake option -DCMAKE_CXX_FLAGS:STRING="-DLLVM_ENABLE_DUMP" -DCMAKE_BUILD_TYPE=RelWithDebInfo.
Details
Diff Detail
Event Timeline
include/llvm/MC/MCSchedule.h | ||
---|---|---|
29 | I don't understand this change. |
include/llvm/MC/MCSchedule.h | ||
---|---|---|
29 | Sorry Mehdi. I know what you mean. In order to remove the "#ifndef NDEBUG" on dumpuses, we also need to remove it here. This patch's goal is to build llvm with cmake option -DCMAKE_CXX_FLAGS:STRING="-DLLVM_ENABLE_DUMP" -DCMAKE_BUILD_TYPE=RelWithDebInfo. First time, I suggested to add "|| defined(LLVM_ENABLE_DUMP)" with "#ifndef NDEBUG" to achieve the goal but I understand what Matthias want. I am not sure how I can satisfy you guys. If possible, can you suggest idea please? I will be very happy with it. |
include/llvm/MC/MCSchedule.h | ||
---|---|---|
29 | This is a case where the dumping code seems to be dependent on !NDEBUG code. You could solve this by tweaking the SchedBoundary::dumpScheduledState() implementation instead. (Like for example not calling getResourceName() with NDEBUG enabled). |
include/llvm/MC/MCSchedule.h | ||
---|---|---|
29 | I appreciate you guys comments. This issue is related to http://bugs.llvm.org/show_bug.cgi?id=32283. I think Chris will manage it overall. If possible, I would like to close this review. |
I don't understand this change.
We don't want to increase the runtime memory consumption when not needed. This should still be compiled out in release build.