Newsgroups: php.internals,php.internals.win
Path: news.php.net
Xref: news.php.net php.internals:89307 php.internals.win:1130
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain realplain.com from 68.114.190.30 cause and error)
Message-ID: <DCDEAA9CDB644DF0AAA540BC4EC1E6E0@pc1>
To: "Anatol Belski" <anatol.php@belski.net>,
	<internals@lists.php.net>,
	<internals-win@lists.php.net>
Cc: "'Dmitry Stogov'" <dmitry@zend.com>,
	"'Pierre Joye'" <pierre.php@gmail.com>
References: <9B23C6783EE5480CB15EAC35B9E420EA@pc1> <031201d12071$c8f66800$5ae33800$@belski.net> <57955F4AAAFF48339CF9194924258215@pc1> <039701d120aa$300afa50$9020eef0$@belski.net>
Date: Mon, 23 Nov 2015 01:15:12 -0600
MIME-Version: 1.0
Content-Type: text/plain;
	format=flowed;
	charset="iso-8859-1";
	reply-type=original
Content-Transfer-Encoding: 7bit
Subject: Re: [INTERNALS-WIN] Re: [PHP-DEV] Windows (Visual Studio) compiler stuff
From: php_lists@realplain.com ("Matt Wilmas")

Hi Anatol, all,

----- Original Message -----
From: "Anatol Belski"
Sent: Monday, November 16, 2015

> Hi Matt,
>
>> -----Original Message-----
>> From: Matt Wilmas [mailto:php_lists@realplain.com]
>> Sent: Monday, November 16, 2015 2:59 PM
>> To: Anatol Belski <anatol.php@belski.net>; internals@lists.php.net;
> internals-
>> win@lists.php.net
>> Cc: 'Dmitry Stogov' <dmitry@zend.com>; 'Pierre Joye'
> <pierre.php@gmail.com>
>> Subject: [INTERNALS-WIN] Re: [PHP-DEV] Windows (Visual Studio) compiler
>> stuff
>>
>> > According to the docs __declspec(noinline) is specific to C++. Also
>> > with VS it's always much more tedious to inline something than the
>> > opposite. These are the main two reasons it's disregarded ATM. We can
>> > add it for compliance with C++, but it'll in best case have no effect
>> > in the PHP core. Should be tested before, though.
>>
>> Yeah, I know what the docs imply ("member function"), which is why I
> tested it.
>> I guess you missed my "works as expected" part. :-P
>>
>> A test function that just returns a number was automatically inlined
> (plain C).
>> Using __declspec(noinline) it was call'ed instead.
>>
>> Not sure if any of the "zend_never_inline" PHP stuff is getting inlined
> when it's
>> desired not to be -- I'll compile PHP in a bit and see what it looks like
> with
>> "noinline."
>>
> Yeah, I knew it could work, just that it's undocumented so preferred not
> even to start with it because I haven't expect much gain from it. The
> functions I've seen with zend_never_inline are rather big and wouldn't get
> inlined even when forced.

noinline did have an effect -- 12 KB smaller php7.dll.  So, obviously it's 
preventing those zend_never_inline functions from being inlined when they 
currently are.  Dmitry surely had reason to make them that way --  
cache-related, I assume.  Any difference, however "minor," is the same as 
other compilers, so it's nice to know this can be used, with so many of the 
other GCC/Clang "tricks" missing...

BTW, something "big" not getting inlined even when forced?  I know the 
"rules" about what can't be [force] inlined (basically same as GCC) and size 
isn't one of them. :-)  (I hope not.)  As I've mentioned a bit, to be seen 
soon, my "compile-time" param parsing optimization will have the "hugest" 
inline function, but it compiles down to literally nothing, which I finally 
got to work with MSVC as well.  That's why I wasn't liking the idea of a 
standalone copy of that stuff adding several KB to each module...

>> > I'd ask you for some concrete case for this, as I'm not sure to
>> > understand exactly what you mean. The only case where an extra code
>> > would be generated is with "__declspec(export) inline", but that's not
>> > the case anywhere within PHP.
>>
>> My concrete case is checking tons of generated code! ;-)
>>
>> It's simple: useless standalone functions are created for every "static
>> __forceinline" definition...  Not having static makes it act like
> GCC/Clang.
>>
> I guess I've understood what you're talking about - abut unreferenced
> COMDATs (or maybe also duplicated COMDATs). There is a variety of 
> situations
> for that, not possibly only inlining. Fixing it is done in PHP when 
> building
> with --enable-debug-pack, that is on in release builds. In your 
> experiments,
> if you add /Zi CFLAG (or explicitly /Gy) and /OPT:REF,ICF LDFLAG - that 
> will
> solve it for yur other project. You can read more about COMDAT on MSDN.

Yeah, I know about the COMDAT stuff.  And I thought I had tried the 
/OPT:REF, etc. on a standalone test awhile ago and it didn't do anything...

I just now tried --enable-debug-pack, and as I was thinking, it had no 
effect.

I don't need to solve anything on the other project since I didn't use 
static there. :-P

> Hm, probably these options could be revisited, as since 2013 there's also
> /Gw and /Zc:inline switches which is not implied by /Zi. But have to do 
> more
> checks, for now the release build options are good enough.
>
>> Again, I'll try to compile PHP with those static's removed and report the
> effect
>> later.
>>
> Yes, thanks for your effort. I actually didn't check what gcc does for 
> such
> cases, so curious. But "static" in "static inline" forces every 
> translation
> unit to have even the same function to have different address, thus
> eliminating the "one definition" rule for inline. We anyway need "static
> inline" best compatibility, the compilers handle the rest :)

First, the report: Removing all the static's with zend_always_inline works 
fine (since the __forceinline seems to "imply" static, no duplicate 
symbols).  It makes php7.dll 91 KB smaller (NTS --disable-all).

But then when I tried the /Zc:inline option (really sounds like C++ on MSDN) 
the other day, I was pleasantly surprised!  "You da man!" :-)

That saved over 220 KB, without removing static's.  I verified that the 
standalone functions (from static's) were gone, but obviously it also 
removed a lot more.  Thank you!

Hopefully that's a switch that can be taken advantage of?

> Regards
>
> Anatol

Thanks,
Matt