[llvm-dev] [LLVMdev] Ideas for making llvm-config --cxxflags more useful

Fri Aug 7 12:32:34 PDT 2015

On Fri, Aug 7, 2015 at 11:30 AM, Pete Cooper <peter_cooper at apple.com> wrote:

> I’ve almost finished a patch to add back in either out of line destructors
> or anchor methods.  We seem to use one or the one, relatively
> inconsistently.
>
> What i’ve gone for is that if a class already had an inline destructor
> then i left it alone and added an anchor method.  Otherwise I added an out
> of line destructor.
>

That seems possibly the opposite of what might be desired - if the class
has no user-declared dtor, it's implicitly inline and possibly trivial (&
doesn't suppress implicit move and copy ops (rule of 0/5)) - I'd avoid
adding a dtor to a class that doesn't have/need one (& whenever I see
empty-bodied dtors I try to remove them - not always possible).

I was probably the first one to use (or at least do a lot of them) the
explicit anchors in an attempt to make LLVM -Wweak-vtables clean but never
got around to going all the way there. I think Chris Lattner even advocated
for not out-of-lining dtors just to make the vtables strong, instead using
an explicit anchor. But I'm pretty ambivalent about it. In theory the style
guide advocates this for build perf reasons - which would be interesting to
test & see if it's actually relevant. If it's a matter of
correctness/functionality as this thread seems to imply (but I haven't read
it enough to really understand what's being dealt with here, to be honest)
then I guess we'll have to decide on some approach... *shrug*

- Dave

>
> Now if I compile Instructions.cpp with -Wweak-vtable, the only warnings
> given are:
>
> *../include/llvm/PassSupport.h:226:8: **warning: **'PassRegistrationListener'
> has no out-of-line virtual method definitions;*
> *      its vtable will be emitted in every translation unit
> [-Wweak-vtables]*
> struct PassRegistrationListener {
> *       ^*
> In file included from ../lib/IR/Instructions.cpp:16:
> In file included from ../lib/IR/LLVMContextImpl.h:19:
> In file included from ../lib/IR/ConstantsContext.h:25:
> *../include/llvm/Support/raw_ostream.h:321:7: **warning: **'raw_pwrite_stream'
> has no out-of-line virtual method*
> *      definitions; its vtable will be emitted in every translation unit
> [-Wweak-vtables]*
> class raw_pwrite_stream : public raw_ostream {
> *      ^*
> *../include/llvm/Support/raw_ostream.h:539:7: **warning: **'buffer_ostream'
> has no out-of-line virtual method definitions;*
> *      its vtable will be emitted in every translation unit
> [-Wweak-vtables]*
> class buffer_ostream : public raw_svector_ostream {
>
> However, I do wonder how useful my patch for Instructions.h (and
> constants.h too which needed work), when you can’t #include PassSupport
> without -fno-rtti.
>
> So, i’l get a patch out soon to look at, but i would like to discuss
> whether any of you think its worth it given the other warnings which still
> prevent no-rtti.
>
> Cheers,
> Pete
>
> On Aug 7, 2015, at 10:54 AM, Pete Cooper via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
> Sorry about this. I think if I'd finished the work to remove vtables from
> Value then it wouldn't be an issue, but I put that on hold due to
> performance concerns.
>
> I can add back in a bunch of anchor functions where needed. Will just need
> to go through and find all the classes which need them.
>
> Pete
>
> Sent from my iPhone
>
> On Aug 7, 2015, at 10:32 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Fri, Aug 7, 2015 at 10:22 AM, Hans Wennborg via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> On Thu, Aug 6, 2015 at 12:04 PM, David Chisnall via llvm-dev
>> <llvm-dev at lists.llvm.org> wrote:
>> > [Ooops, sent to the old list address by mistake]
>> >
>> > On 30 Jul 2015, at 21:04, tom at stellard.net wrote:
>> >>
>> >> For flags like -fno-rtti (are there others?) that are required in some
>> cases
>> >> (I think -fno-rtti is required only if you sub-class LLVM objects), I
>> would propose
>> >> adding a separate flag like --uses-rtti.  This would give users more
>> fine-grained
>> >> control over which flags they use, and also would let them choose the
>> correct
>> >> flag since, for example, -fno-rtti is not understood by MSVC.
>> >
>> > There appears to be a regression in LLVM 3.7, which means that you must
>> compile with -fno-rtti if you include llvm’s Instructions.h.  The issue is
>> that a few of the classes (ICmpInst, GetElementPtrInst and PHINode) are now
>> defined entirely in the header, so every compilation unit that includes the
>> header will emit them.  These classes all inherit from Instruction
>> (indirectly via CmpInst in the case of ICmpInst) and so fail to link if
>> compiled with -fno-rtti, because they can’t find the vtable for ICmpInst.
>>
>> I looked at the file, and this didn't seem true (e.g.
>> GetElementPtrInst::init is still out-of-line). But then I realized you
>> mean virtual functions, so these classes no longer have a key
>> function.
>>
>> This is probably Pete's r240588. I suppose we could add key functions
>> to these classes (even if they're not used for anything). I'm not sure
>> how we'd prevent this from regressing though :-/
>>
>
> In theory the LLVM style guide mandates key functions for all dynamic
> classes (under the claim of build performance - avoiding duplicate vtable
> emission, etc). We've never strongly enforced it though - if we really
> wanted to, we could do so as Clang has a warning that triggers whenever a
> vtable is emitted weakly (which is what happens when there isn't a key
> function).
>
> - David
>
>
>>
>>  - Hans
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org         http://llvm.cs.uiuc.edu
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org         http://llvm.cs.uiuc.edu
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150807/35a2e336/attachment.html>