> 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*
Thanks for the explanation. I knew we wouldn’t be able to inline the destructors when they are out of line, but i didn’t know about the rule of 0/5 which is also interesting.
So I didn’t really like the look of what I did either which is why i thought it best to explain it. I’m happy to change over what I’ve done to anchor’s, but its unfortunate that it will then differ from some of the other sibling classes.
Perhaps this is moot* though, if I can get back to finishing the devirtualisation without any perf issues.
* moot for ToT perhaps. I’m happy to get something for 3.7 if we can work out what it should be. The options seem to be
- Do nothing to the code, except perhaps change llvm-config as David said
- This patch
- The equivalent to this with anchor’s.
Anyone got a strong opinion which is best, especially as I don’t want to do anything too big for 3.7 at this point.
I don't have any strong opinions. Did this actually break anyone's use
case or is it just a hypothetical problem? It doesn't seem like there
is much value in fixing this until llvm-config is also fixed.
The current 3.7 code breaks for the codebase that I use for teaching. It uses RTTI in the AST construction, so can’t be compiled with -fno-rtti, but does need to use Module, IRBuilder, and friends, so not being able to link against an LLVM that’s built without RTTI is annoying (it means that the students can’t use LLVM packages on any OS).
This sounds pretty unfortunate, and if David is running into it then
others probably will too (and those users might find the errors more
If this is possible to fix by adding anchor functions to the various
Instruction subclasses, that seems like a good fix (new functions is
probably better than adding out-of-line ctors/dtors).
In any case, to get such a fix merged to 3.7, the code owner would
need to approve it. I don't think there is a specific owner for the
IR, which means it falls under Chris's catch-all, so I'd like to hear
what he thinks.
(This sub-thread started here:
So, I just raised a related issue on the IRC, and he conclusion was that this email thread is probably the right place to bring this up…
Would it be possible to make llvm-config --cxxflags actually just produce flags essential to compilinging and linking some arbitrary source outside of llvm, without a strict requirement of “identical flags to the ones used by the llvm Makefile”?
In my case, the main concern is that CXX in my Makefile needs to match that which was used to build llvm, since llvm-config --cxxflags produces -Wfoo and -Wbar that is useful for compiler X (clang in my case) but doesn’t “work” in compiler Y (e.g. some version of gcc). So it’s required for my project (that lives outside the llvm tree at the moment) to know which compiler was used to build llvm.
Would it be possible to make llvm-config --cxxflags actually just produce flags essential to compilinging and linking some arbitrary source outside of llvm, without a strict requirement of "identical flags to the ones used by the llvm Makefile”?
It would certainly be a good idea.
In my case, the main concern is that CXX in my Makefile needs to match that which was used to build llvm, since llvm-config --cxxflags produces -Wfoo and -Wbar that is useful for compiler X (clang in my case) but doesn't "work" in compiler Y (e.g. some version of gcc). So it's required for my project (that lives outside the llvm tree at the moment) to know which compiler was used to build llvm.
If those flags silence spurious warnings in headers, then we should probably silence them with a pragma, not a compiler flag, and only in the relevant headers.