<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="">David,<div class=""><br class=""></div><div class="">Thank you for the patch. I’m more than happy to land the patch for you. I’ll commit it within the next hour or so.</div><div class=""><br class=""></div><div class="">Thanks,</div><div class="">-Chris</div><div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Aug 14, 2015, at 2:02 AM, David Wiberg <<a href="mailto:dwiberg@gmail.com" class="">dwiberg@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">2015-08-14 2:13 GMT+02:00 Tom Stellard <</span><a href="mailto:tom@stellard.net" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">tom@stellard.net</a><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">>:</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">On Thu, Aug 13, 2015 at 03:22:54PM -0700, Chris Bieneman wrote:<br class=""><blockquote type="cite" class="">(+Tom Stellard)<br class=""><br 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.<br class=""><br 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.<br class=""><br 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.<br class=""><br 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.<br class=""><br class="">Thoughts?<br class=""></blockquote><br class="">I still don't know enough about CMake to comment on the implementation<br class="">details, but I think a quick fix like this is really the only way we can<br class="">get this fixed in the 3.7 branch, so I would be in favor of merging it.<br class=""><br class="">The correct solution requires some pretty invasive changes, like<br class="">s/NDEBUG/LLVM_NDEBUG/ everywhere, and it will probably need to bake for<br class="">a while in ToT to shake out all the issues.<br class=""><br class="">-Tom<br class=""><br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Thanks for looking at the patch. I didn't push for landing it since I</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">noticed Tom's work which seems to be the correct long-term solution.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">As I mentioned in my initial message I do not have commit rights so I</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">need some help with that part.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Best regards</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">David</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" class="">-Chris<br class=""><br class=""><blockquote type="cite" 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:<br class=""><br class="">Once we have the right solution, this should definitely go to the 3.7 branch.<br class=""><br 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.<br class=""><br class="">On Thu, Aug 13, 2015 at 9:55 AM Hans Wennborg <<a href="mailto:hans@chromium.org" class="">hans@chromium.org</a> <<a href="mailto:hans@chromium.org" class="">mailto:hans@chromium.org</a>>> wrote:<br class="">On Sun, Aug 2, 2015 at 9:27 AM, David Wiberg <<a href="mailto:dwiberg@gmail.com" class="">dwiberg@gmail.com</a> <<a href="mailto:dwiberg@gmail.com" class="">mailto:dwiberg@gmail.com</a>>> wrote:<br class=""><blockquote type="cite" 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=""></blockquote><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=""><blockquote type="cite" 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=""></blockquote><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="">_______________________________________________<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</blockquote></blockquote></blockquote></div></blockquote></div><br class=""></div></body></html>