[PATCH] D11207: Enable recognition of __declspec for PS4

[Aaron Ballman]

On Wed, Jul 15, 2015 at 6:04 PM, David Majnemer
<david.majnemer at gmail.com> wrote:
>
>
> On Wed, Jul 15, 2015 at 2:46 PM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
>>
>> On Wed, Jul 15, 2015 at 5:37 PM, Ristow, Warren
>> <warren_ristow at playstation.sony.com> wrote:
>> >> Is there a reason you cannot use -fms-extensions instead?  ...
>> >
>> > We don't want to enable support of _all_ MS extensions, so just directly
>> > using -fms-extensions isn't viable for us.  You mention __declspec isn't a
>> > Sony extension, but really a "Microsoft (and Borland)" extension.  Along
>> > that line, I'd say one way of looking at this is saying it will now be a
>> > "Microsoft (and Borland and Sony)" extension.
>>
>> I think you missed my point, but I take your meaning just the same.
>> Thank you for the explanation, it's what I figured the case would be.
>>
>> Sony would like support for __declspec without turning on all MS
>> extensions. We currently have the same idea in place to support
>> __declspec for CUDA. It seems to me that there are cases where
>> unconditional support of __declspec is desired. I am wondering if we
>> want an -fdeclspec-ext (or something) that enables just __declspec
>> support, instead of trying to tie this extension to compiler vendors.
>> Obviously, we could turn this extension on for Sony's triple as well
>> in that case. I'm wondering if that's a design we'd want to consider
>> exploring or not.
>
>
> Perhaps we should rethink r238238.  The change was definitely contentious.

I still think 238238 is the correct direction, but I am not opposed to
rethinking it.

I do not think __declspec should be on by default for all consumers of
Clang. I am not opposed to flags enabling it with other language
extensions for various compiler compatibility targets where it makes
sense (Microsoft mode, Borland mode, etc). I am not keen on enabling
it via a flag for non-compiler compatibility targets (CUDA, Sony,
etc). We used to allow this feature unconditionally and the world
didn't burn, so providing a knob to turn just this feature on seems
reasonable, and it addresses my concerns with non-compiler
compatibility targets (which can turn the feature on by default, but
now have a way to disable the feature when wanting more conforming
code).

~Aaron

>
>>
>>
>> ~Aaron
>>
>> >
>> > -Warren
>> >
>> > -----Original Message-----
>> > From: aaron.ballman at gmail.com [mailto:aaron.ballman at gmail.com] On Behalf
>> > Of Aaron Ballman
>> > Sent: Wednesday, July 15, 2015 11:48 AM
>> > To: Ristow, Warren
>> > Cc: reviews+D11207+public+3fbdfe66c3b43ce8 at reviews.llvm.org; Anton
>> > Korobeynikov; llvm cfe
>> > Subject: Re: [PATCH] D11207: Enable recognition of __declspec for PS4
>> >
>> > On Wed, Jul 15, 2015 at 2:07 PM, Ristow, Warren
>> > <warren_ristow at playstation.sony.com> wrote:
>> >>> Can I get a brief description of what you're trying to accomplish and
>> >>> why this is needed?
>> >>
>> >> We intend to support __declspec(dllimport)/__declspec(dllexport) on
>> >> PS4, and we will be uploading code reviews for that.  So we're enabling
>> >> recognition of __declspec on PS4 as a prerequisite for that.
>> >
>> > Is there a reason you cannot use -fms-extensions instead? I'm a little
>> > uncomfortable with the line being drawn here because this isn't a Sony
>> > extension, it's a Microsoft (and Borland) one where we are trying to
>> > be compatible with what those compilers do. For instance, this opens
>> > the door for Sony-specific behavior of a Microsoft compiler feature
>> > that Clang supports. However, I may also be out of the loop on why
>> > this is needed. :-)
>> >
>> > ~Aaron
>> >
>> >>
>> >> -Warren
>> >>
>> >> -----Original Message-----
>> >> From: aaron.ballman at gmail.com [mailto:aaron.ballman at gmail.com] On
>> >> Behalf Of Aaron Ballman
>> >> Sent: Wednesday, July 15, 2015 10:49 AM
>> >> To: reviews+D11207+public+3fbdfe66c3b43ce8 at reviews.llvm.org
>> >> Cc: Ristow, Warren; Anton Korobeynikov; llvm cfe
>> >> Subject: Re: [PATCH] D11207: Enable recognition of __declspec for PS4
>> >>
>> >> On Tue, Jul 14, 2015 at 7:36 PM, Warren Ristow
>> >> <warren_ristow at playstation.sony.com> wrote:
>> >>> wristow created this revision.
>> >>> wristow added a reviewer: asl.
>> >>> wristow added a subscriber: cfe-commits.
>> >>>
>> >>> This change includes the changes of reviews.llvm.org/D11164 (reserving
>> >>> a flag for Sony/PS extensions), and also uses that flag to enable
>> >>> recognition of __declspec.  This supersedes D11164.
>> >>>
>> >>> Can someone review, and if OK then commit this patch for me, please?
>> >>
>> >> Can I get a brief description of what you're trying to accomplish and
>> >> why this is needed?
>> >>
>> >> ~Aaron
>> >>
>> >>>
>> >>> -Warren Ristow
>> >>> SN Systems - Sony Computer Entertainment Group
>> >>>
>> >>> http://reviews.llvm.org/D11207
>> >>>
>> >>> Files:
>> >>>   include/clang/Basic/LangOptions.def
>> >>>   include/clang/Basic/TokenKinds.def
>> >>>   include/clang/Driver/CC1Options.td
>> >>>   include/clang/Parse/Parser.h
>> >>>   lib/Basic/IdentifierTable.cpp
>> >>>   lib/Driver/Tools.cpp
>> >>>   lib/Frontend/CompilerInvocation.cpp
>> >>>   lib/Parse/ParseDecl.cpp
>> >>>   test/Lexer/keywords_test.c
>> >>>   test/Lexer/keywords_test.cpp
>> >>>
>> >>>
>> >>> _______________________________________________
>> >>> cfe-commits mailing list
>> >>> cfe-commits at cs.uiuc.edu
>> >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>> >>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>