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

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150807/39d33b4c/attachment.html>

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
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150807/857d9d4c/attachment.html>

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.

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
<mailto: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 <mailto: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 <mailto: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 <mailto: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 <mailto:llvm-dev at lists.llvm.org>   
http://llvm.cs.uiuc.edu <http://llvm.cs.uiuc.edu/>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<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/a1382a5c/attachment.html>