> On Jul 17, 2015, at 11:46 AM, David Majnemer <david.majne...@gmail.com> wrote:
>
>
>
> On Fri, Jul 17, 2015 at 11:17 AM, Bob Wilson <bob.wil...@apple.com
> <mailto:bob.wil...@apple.com>> wrote:
>
>> On Jul 17, 2015, at 10:40 AM, David Majnemer <david.majne...@gmail.com
>> <mailto:david.majne...@gmail.com>> wrote:
>>
>>
>>
>> On Fri, Jul 17, 2015 at 10:38 AM, Bob Wilson <bob.wil...@apple.com
>> <mailto:bob.wil...@apple.com>> wrote:
>>
>>> On Jul 17, 2015, at 10:34 AM, David Majnemer <david.majne...@gmail.com
>>> <mailto:david.majne...@gmail.com>> wrote:
>>>
>>>
>>>
>>> On Fri, Jul 17, 2015 at 10:25 AM, Bob Wilson <bob.wil...@apple.com
>>> <mailto:bob.wil...@apple.com>> wrote:
>>>
>>>> On Jul 17, 2015, at 10:17 AM, Reid Kleckner <r...@google.com
>>>> <mailto:r...@google.com>> wrote:
>>>>
>>>> On Fri, Jul 17, 2015 at 4:45 AM, Aaron Ballman <aa...@aaronballman.com
>>>> <mailto:aa...@aaronballman.com>> wrote:
>>>> On Thu, Jul 16, 2015 at 8:42 PM, Bob Wilson <bob.wil...@apple.com
>>>> <mailto:bob.wil...@apple.com>> wrote:
>>>> > Clang used to silently ignore __declspec(novtable) for all platforms,
>>>> > but it is now implemented for Windows. However, we don’t check if the
>>>> > target is Windows. This does not work when using the Itanium ABI, where
>>>> > the class layout for complex class hierarchies is stored in the vtable.
>>>> > Leaving the vtable uninitialized on non-Windows platforms does not work
>>>> > in that case. It might be possible to honor the novtable attribute in
>>>> > some simple cases and either report an error or ignore it in more
>>>> > complex situations, but it’s not clear if that would be worthwhile.
>>>> > There is also value in having a simple and predictable behavior, so I am
>>>> > proposed the attached patch which simply ignores novtable on non-Windows
>>>> > platforms.
>>>>
>>>> I think it might actually be worth making it work. I have vague
>>>> recollections of Chromium developers wondering how to do the equivalent
>>>> size saving optimization on non-Windows targets. We'd have to pin down
>>>> what makes a "complex" class hierarchy. I'm assuming the fix would be to
>>>> emit the vptr store if the class has virtual bases.
>>>>
>>>> MSVC supports an Itanium build target. What does __declspec(novtable)
>>>> do there with the complex class layouts?
>>>>
>>>> I don't have Visual Studio installed with support for Itanium,
>>>> otherwise I would test this myself.
>>>>
>>>> I think Bob is talking about the Itanium C++ ABI, which I don't think MSVC
>>>> ever implemented. If they did, I wouldn't be surprised if they simply
>>>> ignored this declspec.
>>>
>>> Yes — I meant the Itanium C++ ABI.
>>>
>>> If someone else wants to sign up to implement this properly, I have no
>>> objections. In the meantime, my patch would fix the miscompiles that result
>>> from the current behavior. I’d still like to go ahead with it.
>>>
>>> My only qualm with the patch is that it wouldn't engage for MingW targets.
>>> It LGTM but the predicate needs tweaking to focus on the MSVC compatible
>>> targets..
>>
>> That makes sense. The “TargetWindows” predicate is also used for the
>> dllexport and dllimport declspecs. Would it make sense to treat those in the
>> same way? It has been a while since I looked at MinGW.
>>
>> MingW needs dllimport and dllexports so its OK for them to be using the
>> predicate.
>
> I looked into changing this, but there’s nothing to be done. The canonical
> representation of MinGW in the target triples is *-*-windows-gnu, and the
> predicate is checking the OS component.
>
> Right, we need a new predicate keyed off of
> llvm::Triple::isKnownWindowsMSVCEnvironment.
You want novtable to be ignored for MinGW? (Sorry — I’ve used MinGW in the
past, but I don’t know what C++ ABI it uses.)
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
