Hi Anatol,
----- Original Message -----
From: "Anatol Belski"
Sent: Wednesday, November 25, 2015
Hi Matt,
-----Original Message-----
From: Matt Wilmas [mailto:[email protected]]
Sent: Monday, November 23, 2015 8:15 AM
To: Anatol Belski <[email protected]>; [email protected];
internals-
[email protected]
Cc: 'Dmitry Stogov' <[email protected]>; 'Pierre Joye'
<[email protected]>
Subject: Re: [INTERNALS-WIN] Re: [PHP-DEV] Windows (Visual Studio)
compiler
stuff
Hi Anatol, all,
----- Original Message -----
From: "Anatol Belski"
Sent: Monday, November 16, 2015
[...]
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...
I wasn't telling it wouldn't work. We should check for possible
implications. If there's nothing negative, so we can add this into master.
It always depends, smaller image size vs. function call.
It works and I don't see how there could be implications, so "do it" I say.
;-)
It doesn't really depend in the case of zend_never_inline -- the *point* is
to ensure a function call (and smaller code size). Question/take it up with
Dmitry otherwise. :-P
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...
Size is one of the factors, the concrete code and usage, too. Despite
that,
any compiler doc says that inline is just a suggestion.
This is unrelated to anything anyway, but... We're not talking about "just
inline" here, but always/force. Much more than "just a suggestion." At
least when optimization is enabled, it WILL be inlined provided it doesn't
contain one of the things that makes it ineligible for inlining.
So it's more like Arnold in T2: "I insist." Or, a very strong suggestion.
> 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 a while ago and it didn't do anything...
I just now tried --enable-debug-pack, and as I was thinking, it had no
effect.
What do you mean with "no effect"? Don't reduce size? The compiler/linker
options I've mentioned are about removing identical or unreferenced
COMDATS,
and they do that. BTW how do you check it? I would like you to be more
precise at this point, please. Did you use link /map or disasm?
No effect meaning it didn't do anything at all. So no, they didn't remove
the, what, 220 KB+ worth of code... File size identical, so I assume all
contents as well (except image headers, etc.).
I've always been checking file size (for quick answer) and disassembly...
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?
/Zc:inline is documented as C++11 feature. Still it is about enforcing the
definition within the same translation unit, so basically kind of
synonymous
to the cl/link options we have. It doesn't enforce C++11, just one that
rule. Whether it'd break some C++ extensions - well, should check.
It sounded to me (haven't checked docs again) like it's something that sets
whether old, MS-specific behavior is disallowed...? e.g. nothing to break
if it didn't [already] break with other compilers.
But about the "static inline", it is really something that should be kept
everywhere. It is the most convenient option for the compiler/linker
compatibility. A global function is allowed to be defined only once. Since
those functions are in defined in the headers, chances are to see the
duplicated symbol errors which will prevent compilation. VC should
actually
should do same.
I'm no longer asking about removing the "static" part anywhere, like
original message, since extra junk can be removed with /Zc:inline. :-)
I get how the function definition stuff works, but VC doesn't seem to! As
I've already said, by default (e.g. without /Zc:inline):
*) __forceinline already seems to imply "static" by itself. Everything
works as expected, all compilation works fine, and there are never the
would-be-expected duplicate symbol errors.
*) adding "static" to __forceinline, again, by default, seems to make it
"super static" and create separate standalone versions even though they are
never referenced.
Seems there's a bug there somewhere...
The option using "extern inline" and splitting declaration
and definition are unusable, because those functions have to be usable in
external modules.
In general, if testing goes good, we could add these options
(__declspec(noinline), /Zc:inline and maybe /Gw) to master to release
builds
for further observation. But it should be really good tested. We'll check
it
in our labs as well. I'll be able to come to this topic either at the end
of
this or early next year.
MSVC really has problems if the options mess anything up ;-), so I'm making
the changes as I find them, hehe.
Regards
Anatol
- Matt
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
