[PATCH] D38406: [dump] Remove NDEBUG from test to enable dump methods [NFC]

Don Hinton via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 4 19:02:34 PST 2017


hintonda added a comment.

In https://reviews.llvm.org/D38406#944588, @MatzeB wrote:

> In https://reviews.llvm.org/D38406#944576, @hintonda wrote:
>
> > Re-opening:
> >
> > This was reverted in https://reviews.llvm.org/rL315854 due to bot failures using multi-configuration generators, e.g., XCode and MSVC.
> >
> > There were 2 issues:
> >
> > 1. LLVM_ENABLE_DEBUG was missing on some dump declartions,
> > 2. LLVM_ENABLE_DEBUG was set based on CMAKE_BUILD_TYPE which isn't set when using multi-configuration generators.
> >
> >   We can fix 1 as needed, and 2 by adding LLVM_ENABLE_DUMP to CMAKE_CXX_FLAGS_(RELEASE|MINSIZEREL) as needed.
>
>
> Just some brainstorming for the generator problems (didn't really check if cmake is flexible enough to pull this off): Would it be possible to do cmake hackery to create a directory for every CMAKE_BUILD_TYPE and then generate the llvm-config.h apropriate for each BUILT_TYPE into each directory and add an -I flag to the flags of each build type.


Config directories are already created, e.g.: Release, Debug, etc., with bin and lib subdirs in each.  We could easily add an include subdir to each, but then it gets complicated with adding -I as you mentioned.

But since the CMAKE_CXX_FLAGS_<config> variable already handles this, and also works for the case where headers don't include any other headers, I think it's a more general solution -- this was brought up by Justin Bogner earlier this evening, e.g.: MC/MCSchedule.h.


https://reviews.llvm.org/D38406





More information about the llvm-commits mailing list