<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">(+Tom Stellard)<div class=""><br class=""></div><div class="">Looking at the patches, they’re probably the right way to go as a temporary fix. Certainly this is the change that should go into 3.7, so if we want this for 3.7 we should push it there. Any better fix will be bigger and riskier.</div><div class=""><br class=""></div><div class="">Tom has been working on improving llvm-config recently, and has sent out a batch of patches (sorry for not getting to review them yet). In general I think Tom’s solution is closer to what we actually want to end up with.</div><div class=""><br class=""></div><div class="">The code path in these patches makes me feel a little dirty because it uses get_property to read cxx flags of the target, but we set those flags based on variables in the llvm_update_compile_flags function. That whole flow just feels wrong to me, but I don’t think there is a simple solution. A better solution will probably involve refactoring (and maybe eliminating) llvm_update_compile_flags.</div><div class=""><br class=""></div><div class="">TL;DR: These patches are fine to land, but we really should solve this in a better way in the not-so-distant future.</div><div class=""><br class=""></div><div class="">Thoughts?</div><div class="">-Chris</div><div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Aug 13, 2015, at 2:43 PM, Chandler Carruth via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">Once we have the right solution, this should definitely go to the 3.7 branch.<div class=""><br class=""></div><div class="">I'm not sure this is the best way to get the compile flags though. CC-ing Chris as he's probably the most immersed in this side of CMake at the moment.</div><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Thu, Aug 13, 2015 at 9:55 AM Hans Wennborg <<a href="mailto:hans@chromium.org" class="">hans@chromium.org</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Sun, Aug 2, 2015 at 9:27 AM, David Wiberg <<a href="mailto:dwiberg@gmail.com" target="_blank" class="">dwiberg@gmail.com</a>> wrote:<br class="">
> Hi,<br class="">
><br class="">
> I took a stab at solving PR14200. My CMake knowledge is low but I think the<br class="">
> solution is reasonable as it reuses the existing logic for setting flags.<br class="">
<br class="">
Alexey: you're a cmake wizard. Can you take a look at this?<br class="">
<br class="">
Chandler: you're the cmake owner. I'd like to merge this to 3.7 when<br class="">
it lands if that's OK.<br class="">
<br class="">
> Things worth noting during review:<br class="">
> - The change means that both RTTI and exception handling flags will be<br class="">
> reported by llvm-config.<br class="">
> - I'm creating a variable with the same name as a target property. I don't<br class="">
> know if this breaks any convention.<br class="">
<br class="">
I tried this locally, and the output of llvm-config --cxxflags changed from:<br class="">
<br class="">
  -I/work/llvm/include -I/work/llvm/build.release/include  -fPIC<br class="">
-fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter<br class="">
-Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic<br class="">
-Wno-long-long -Wno-maybe-uninitialized -Wno-comment -std=c++11<br class="">
-ffunction-sections -fdata-sections -O3   -D_GNU_SOURCE<br class="">
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS<br class="">
<br class="">
to:<br class="">
<br class="">
  -I/work/llvm/include -I/work/llvm/build.release/include  -fPIC<br class="">
-fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter<br class="">
-Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic<br class="">
-Wno-long-long -Wno-maybe-uninitialized -Wno-comment -std=c++11<br class="">
-ffunction-sections -fdata-sections -O3  -fno-exceptions -fno-rtti<br class="">
-D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS<br class="">
-D__STDC_LIMIT_MACROS<br class="">
<br class="">
which seems correct.<br class="">
<br class="">
Thanks,<br class="">
Hans<br class="">
</blockquote></div></div>
_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a><br class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits<br class=""></div></blockquote></div><br class=""></div></body></html>