[PATCH][CMake] Fix PR14200, llvm-config output misses -fno-rtti

Fri Aug 14 09:24:21 PDT 2015

I committed this in r245064.

Assuming there are no issues that arise on the bots this should be safe to bring back to 3.7.

David, thank you again for this patch.

-Chris

> On Aug 14, 2015, at 8:45 AM, Chris Bieneman via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> David,
> 
> 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.
> 
> Thanks,
> -Chris
> 
>> On Aug 14, 2015, at 2:02 AM, David Wiberg <dwiberg at gmail.com <mailto:dwiberg at gmail.com>> wrote:
>> 
>> 2015-08-14 2:13 GMT+02:00 Tom Stellard <tom at stellard.net <mailto:tom at stellard.net>>:
>>> On Thu, Aug 13, 2015 at 03:22:54PM -0700, Chris Bieneman wrote:
>>>> (+Tom Stellard)
>>>> 
>>>> 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.
>>>> 
>>>> 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.
>>>> 
>>>> 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.
>>>> 
>>>> TL;DR: These patches are fine to land, but we really should solve this in a better way in the not-so-distant future.
>>>> 
>>>> Thoughts?
>>> 
>>> I still don't know enough about CMake to comment on the implementation
>>> details, but I think a quick fix like this is really the only way we can
>>> get this fixed in the 3.7 branch, so I would be in favor of merging it.
>>> 
>>> The correct solution requires some pretty invasive changes, like
>>> s/NDEBUG/LLVM_NDEBUG/ everywhere, and it will probably need to bake for
>>> a while in ToT to shake out all the issues.
>>> 
>>> -Tom
>>> 
>> 
>> Thanks for looking at the patch. I didn't push for landing it since I
>> noticed Tom's work which seems to be the correct long-term solution.
>> As I mentioned in my initial message I do not have commit rights so I
>> need some help with that part.
>> 
>> Best regards
>> David
>> 
>>>> -Chris
>>>> 
>>>>> On Aug 13, 2015, at 2:43 PM, Chandler Carruth via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>>>>> 
>>>>> Once we have the right solution, this should definitely go to the 3.7 branch.
>>>>> 
>>>>> 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.
>>>>> 
>>>>> On Thu, Aug 13, 2015 at 9:55 AM Hans Wennborg <hans at chromium.org <mailto:hans at chromium.org> <mailto:hans at chromium.org <mailto:hans at chromium.org>>> wrote:
>>>>> On Sun, Aug 2, 2015 at 9:27 AM, David Wiberg <dwiberg at gmail.com <mailto:dwiberg at gmail.com> <mailto:dwiberg at gmail.com <mailto:dwiberg at gmail.com>>> wrote:
>>>>>> Hi,
>>>>>> 
>>>>>> I took a stab at solving PR14200. My CMake knowledge is low but I think the
>>>>>> solution is reasonable as it reuses the existing logic for setting flags.
>>>>> 
>>>>> Alexey: you're a cmake wizard. Can you take a look at this?
>>>>> 
>>>>> Chandler: you're the cmake owner. I'd like to merge this to 3.7 when
>>>>> it lands if that's OK.
>>>>> 
>>>>>> Things worth noting during review:
>>>>>> - The change means that both RTTI and exception handling flags will be
>>>>>> reported by llvm-config.
>>>>>> - I'm creating a variable with the same name as a target property. I don't
>>>>>> know if this breaks any convention.
>>>>> 
>>>>> I tried this locally, and the output of llvm-config --cxxflags changed from:
>>>>> 
>>>>>  -I/work/llvm/include -I/work/llvm/build.release/include  -fPIC
>>>>> -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter
>>>>> -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic
>>>>> -Wno-long-long -Wno-maybe-uninitialized -Wno-comment -std=c++11
>>>>> -ffunction-sections -fdata-sections -O3   -D_GNU_SOURCE
>>>>> -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
>>>>> 
>>>>> to:
>>>>> 
>>>>>  -I/work/llvm/include -I/work/llvm/build.release/include  -fPIC
>>>>> -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter
>>>>> -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic
>>>>> -Wno-long-long -Wno-maybe-uninitialized -Wno-comment -std=c++11
>>>>> -ffunction-sections -fdata-sections -O3  -fno-exceptions -fno-rtti
>>>>> -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS
>>>>> -D__STDC_LIMIT_MACROS
>>>>> 
>>>>> which seems correct.
>>>>> 
>>>>> Thanks,
>>>>> Hans
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150814/d7495fe2/attachment.html>