Windows (Visual Studio) compiler stuff

9 years ago by Matt Wilmas — view source

unread

Hi Dmitry, Anatol, Pierre (etc.), and all,

I'm back now, I think, after a much longer (unintentional) break than I
expected. Be coming very soon with what I was doing in the summer (param
parsing stuff) -- now it works with MSVC too, barring any fragility, as I
accidentally discovered last month...

I've been "discovering" a lot with the wacky Visual Studio compiler! :-)
This message is about the 2 I found today.

The first simple thing was probably just overlooked, but noticed it while
looking up __declspec. zend_never_inline has always been empty (I guess)
for MSVC, but there's actually a __declspec(noinline) that can be used (and
works as expected). A simple and obvious change to bring it in line with
the other compilers?

The second "issue" is with the zend_always_inline functions, I noticed this
summer. Did anyone else know that MSVC leaves a copy of those functions
in the output files (DLLs)? What's the point of that? When they've been
inlined, and not referenced otherwise, there should be no reason to emit
code for a standalone function!

I remembered after seeing that behavior that a bit of my own __forceinline'd
code did NOT have extra function code, but forgot to investigate until
today. What's different about my function definition? No "static"
specifier! So that's the key. :-)

But... non-static would create duplicate symbols, I thought. But no, it
works! With just __forceinline, there's no errors. :^)

Can something be done about this? It would cut the binary size down a bit.
A zend_static macro to be used with zend_always_inline...?

Note: I didn't compile PHP, just quick standalone tests to check that
"noinline" works, no useless functions, and no link error. Both VS 2008 &
2015 (same results).

Thoughts?

Thanks,
Matt

9 years ago by Dmitry Stogov — view source

unread

Hi Matt,

On Mon, Nov 16, 2015 at 1:30 AM, Matt Wilmas php_lists@realplain.com
wrote:

Hi Dmitry, Anatol, Pierre (etc.), and all,

I'm back now, I think, after a much longer (unintentional) break than I
expected. Be coming very soon with what I was doing in the summer (param
parsing stuff) -- now it works with MSVC too, barring any fragility, as I
accidentally discovered last month...

I've been "discovering" a lot with the wacky Visual Studio compiler! :-)
This message is about the 2 I found today.

The first simple thing was probably just overlooked, but noticed it while
looking up __declspec. zend_never_inline has always been empty (I guess)
for MSVC, but there's actually a __declspec(noinline) that can be used (and
works as expected). A simple and obvious change to bring it in line with
the other compilers?

Please, provide a patch for zend_portability.h

The second "issue" is with the zend_always_inline functions, I noticed
this summer. Did anyone else know that MSVC leaves a copy of those
functions in the output files (DLLs)? What's the point of that? When
they've been inlined, and not referenced otherwise, there should be no
reason to emit code for a standalone function!

I remembered after seeing that behavior that a bit of my own
__forceinline'd code did NOT have extra function code, but forgot to
investigate until today. What's different about my function definition?
No "static" specifier! So that's the key. :-)

But... non-static would create duplicate symbols, I thought. But no, it
works! With just __forceinline, there's no errors. :^)

Can something be done about this? It would cut the binary size down a
bit. A zend_static macro to be used with zend_always_inline...?

zend_always inline should be always used with static keyword.

Thanks. Dmitry.

Note: I didn't compile PHP, just quick standalone tests to check that
"noinline" works, no useless functions, and no link error. Both VS 2008 &
2015 (same results).

Thoughts?

Thanks,
Matt

9 years ago by Matt Wilmas — view source

unread

Hi Dmitry,

----- Original Message -----
From: "Dmitry Stogov"
Sent: Monday, November 16, 2015

Hi Matt,

On Mon, Nov 16, 2015 at 1:30 AM, Matt Wilmas php_lists@realplain.com
wrote:

Hi Dmitry, Anatol, Pierre (etc.), and all,

I'm back now, I think, after a much longer (unintentional) break than I
expected. Be coming very soon with what I was doing in the summer (param
parsing stuff) -- now it works with MSVC too, barring any fragility, as
I
accidentally discovered last month...

I've been "discovering" a lot with the wacky Visual Studio compiler! :-)
This message is about the 2 I found today.

The first simple thing was probably just overlooked, but noticed it while
looking up __declspec. zend_never_inline has always been empty (I guess)
for MSVC, but there's actually a __declspec(noinline) that can be used
(and
works as expected). A simple and obvious change to bring it in line with
the other compilers?

Please, provide a patch for zend_portability.h

Really? :-) And that's easier than just adding

__declspec(noinline)

to line 287?

I can add it myself soon, if it's acceptable. And yes, I verified that it
does have an effect, and prevents current inlining, as you intended. (12 KB
smaller binary.)

The second "issue" is with the zend_always_inline functions, I noticed
this summer. Did anyone else know that MSVC leaves a copy of those
functions in the output files (DLLs)? What's the point of that? When
they've been inlined, and not referenced otherwise, there should be no
reason to emit code for a standalone function!

I remembered after seeing that behavior that a bit of my own
__forceinline'd code did NOT have extra function code, but forgot to
investigate until today. What's different about my function definition?
No "static" specifier! So that's the key. :-)

But... non-static would create duplicate symbols, I thought. But no, it
works! With just __forceinline, there's no errors. :^)

Can something be done about this? It would cut the binary size down a
bit. A zend_static macro to be used with zend_always_inline...?

zend_always inline should be always used with static keyword.

Right, that would be the standard thinking, but it makes MSVC not remove the
unreferenced standalone function like GCC/Clang... Adds almost 100 KB to
php7.dll.

But nevermind... Thanks to Anatol, it seems another compiler flag WILL
actually strip those functions, and then some! :-)

Thanks. Dmitry.

Matt

9 years ago by Dmitry Stogov — view source

unread

On Mon, Nov 23, 2015 at 8:08 AM, Matt Wilmas php_lists@realplain.com
wrote:

Hi Dmitry,

----- Original Message -----
From: "Dmitry Stogov"
Sent: Monday, November 16, 2015

Hi Matt,

On Mon, Nov 16, 2015 at 1:30 AM, Matt Wilmas php_lists@realplain.com
wrote:

Hi Dmitry, Anatol, Pierre (etc.), and all,

I'm back now, I think, after a much longer (unintentional) break than I
expected. Be coming very soon with what I was doing in the summer (param
parsing stuff) -- now it works with MSVC too, barring any fragility,
as I
accidentally discovered last month...

I've been "discovering" a lot with the wacky Visual Studio compiler! :-)
This message is about the 2 I found today.

The first simple thing was probably just overlooked, but noticed it while
looking up __declspec. zend_never_inline has always been empty (I guess)
for MSVC, but there's actually a __declspec(noinline) that can be used
(and
works as expected). A simple and obvious change to bring it in line with
the other compilers?

Please, provide a patch for zend_portability.h

Really? :-) And that's easier than just adding

__declspec(noinline)

to line 287?

I can add it myself soon, if it's acceptable. And yes, I verified that it
does have an effect, and prevents current inlining, as you intended. (12
KB smaller binary.)

I don't test Windows builds.
If this OK for Anatol - just commit this.

Thanks. Dmitry.

The second "issue" is with the zend_always_inline functions, I noticed

this summer. Did anyone else know that MSVC leaves a copy of those
functions in the output files (DLLs)? What's the point of that? When
they've been inlined, and not referenced otherwise, there should be no
reason to emit code for a standalone function!

I remembered after seeing that behavior that a bit of my own
__forceinline'd code did NOT have extra function code, but forgot to
investigate until today. What's different about my function definition?
No "static" specifier! So that's the key. :-)

But... non-static would create duplicate symbols, I thought. But no, it
works! With just __forceinline, there's no errors. :^)

Can something be done about this? It would cut the binary size down a
bit. A zend_static macro to be used with zend_always_inline...?

zend_always inline should be always used with static keyword.

Right, that would be the standard thinking, but it makes MSVC not remove
the unreferenced standalone function like GCC/Clang... Adds almost 100 KB
to php7.dll.

But nevermind... Thanks to Anatol, it seems another compiler flag WILL
actually strip those functions, and then some! :-)

Thanks. Dmitry.

Matt

9 years ago by Anatol Belski — view source

unread

Hello Matt,

-----Original Message-----
From: Matt Wilmas [mailto:php_lists@realplain.com]
Sent: Sunday, November 15, 2015 11:31 PM
To: internals@lists.php.net; internals-win@lists.php.net
Cc: Dmitry Stogov dmitry@zend.com; Anatol Belski
anatol.php@belski.net;
Pierre Joye pierre.php@gmail.com
Subject: [PHP-DEV] Windows (Visual Studio) compiler stuff

Hi Dmitry, Anatol, Pierre (etc.), and all,

I'm back now, I think, after a much longer (unintentional) break than I
expected.
Be coming very soon with what I was doing in the summer (param parsing
stuff)
-- now it works with MSVC too, barring any fragility, as I accidentally
discovered last month...

I've been "discovering" a lot with the wacky Visual Studio compiler! :-)
This
message is about the 2 I found today.

The first simple thing was probably just overlooked, but noticed it while
looking
up __declspec. zend_never_inline has always been empty (I guess) for
MSVC,
but there's actually a __declspec(noinline) that can be used (and works as
expected). A simple and obvious change to bring it in line with the other
compilers?

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.

The second "issue" is with the zend_always_inline functions, I noticed
this
summer. Did anyone else know that MSVC leaves a copy of those functions
in
the output files (DLLs)? What's the point of that? When they've been
inlined,
and not referenced otherwise, there should be no reason to emit code for a
standalone function!

I remembered after seeing that behavior that a bit of my own
__forceinline'd
code did NOT have extra function code, but forgot to investigate until
today.
What's different about my function definition? No "static"
specifier! So that's the key. :-)

But... non-static would create duplicate symbols, I thought. But no, it
works!
With just __forceinline, there's no errors. :^)

Can something be done about this? It would cut the binary size down a
bit.
A zend_static macro to be used with zend_always_inline...?

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.

Regards

Anatol

9 years ago by Matt Wilmas — view source

unread

Hi Anatol,

----- Original Message -----
From: "Anatol Belski"
Sent: Monday, November 16, 2015

Hello Matt,

-----Original Message-----
From: Matt Wilmas [mailto:php_lists@realplain.com]
Sent: Sunday, November 15, 2015 11:31 PM
To: internals@lists.php.net; internals-win@lists.php.net
Cc: Dmitry Stogov dmitry@zend.com; Anatol Belski
anatol.php@belski.net;
Pierre Joye pierre.php@gmail.com
Subject: [PHP-DEV] Windows (Visual Studio) compiler stuff

Hi Dmitry, Anatol, Pierre (etc.), and all,

I'm back now, I think, after a much longer (unintentional) break than I
expected.
Be coming very soon with what I was doing in the summer (param parsing
stuff)
-- now it works with MSVC too, barring any fragility, as I accidentally
discovered last month...

I've been "discovering" a lot with the wacky Visual Studio compiler! :-)
This
message is about the 2 I found today.

The first simple thing was probably just overlooked, but noticed it while
looking
up __declspec. zend_never_inline has always been empty (I guess) for
MSVC,
but there's actually a __declspec(noinline) that can be used (and works
as
expected). A simple and obvious change to bring it in line with the
other
compilers?

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."

The second "issue" is with the zend_always_inline functions, I noticed
this
summer. Did anyone else know that MSVC leaves a copy of those
functions
in
the output files (DLLs)? What's the point of that? When they've been
inlined,
and not referenced otherwise, there should be no reason to emit code for
a
standalone function!

I remembered after seeing that behavior that a bit of my own
__forceinline'd
code did NOT have extra function code, but forgot to investigate until
today.
What's different about my function definition? No "static"
specifier! So that's the key. :-)

But... non-static would create duplicate symbols, I thought. But no, it
works!
With just __forceinline, there's no errors. :^)

Can something be done about this? It would cut the binary size down a
bit.
A zend_static macro to be used with zend_always_inline...?

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.

Again, I'll try to compile PHP with those static's removed and report the
effect later.

Regards

Anatol

Matt

9 years ago by Anatol Belski — view source

unread

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.

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.

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 :)

Regards

Anatol

9 years ago by Matt Wilmas — view source

unread

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

9 years ago by Anatol Belski — view source

unread

Hi Matt,

-----Original Message-----
From: Matt Wilmas [mailto:php_lists@realplain.com]
Sent: Monday, November 23, 2015 8:15 AM
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: Re: [INTERNALS-WIN] Re: [PHP-DEV] Windows (Visual Studio)
compiler
stuff

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...

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.

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.

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 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?

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.

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. 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.

Regards

Anatol

8 years ago by Matt Wilmas — view source

unread

Hi Anatol,

----- Original Message -----
From: "Anatol Belski"
Sent: Wednesday, November 25, 2015

Hi Matt,

-----Original Message-----
From: Matt Wilmas [mailto:php_lists@realplain.com]
Sent: Monday, November 23, 2015 8:15 AM
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: 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

9 years ago by Matt Wilmas — view source

unread

Hi Anatol, Dmitry, all,

Will reply about the original subject issues soon, but this is about new
stuff I noticed the other day... Adding Matt Tait and Nikita because of PR
#1418 and comments.

Anyway, the new Control Flow Guard (/guard:cf) is causing a big slowdown on
bench.php. :-( 14% on a Yorkfield (Q9400) and 19% on a Sandy Bridge
(Celeron G530). Ouch. Did anyone else check the performance impact? Is
this acceptable? On any other platform...?

I'll definitely remove that from my builds (Elephpant Sanctuary, coming
soon) since it's useless on all but the latest Windows versions anyway.

But if that "feature" must remain enabled otherwise, I think we can
eliminate most of the performance hit. As Nikita wondered about, I first
wanted to look at the indirect calls to the opcode handlers. I tried
separating out zend_execute.c in the Makefile and added /guard:cf- Bingo!
That restored about 98% of the speed on bench.php. It reduced
the --disable-all NTS DLL by 13.5 KB (of the 67 KB added by full CFG).

Or could maybe change back to the old SWITCH executor? I didn't try that.

It seems like it would be a good "rule" to not use any MS stuff that isn't
done on other compilers/platforms. :-)

/GS [1] is another that is/was starting to get annoying (function
prolog/epilog); luckily I was able to suppress it in most cases with changes
I'm making. It's enabled by default, of course, although I see it's
commented out in a line (old?) of confutils.js. /GS- ;-) I really hope
there aren't places where we are not doing range checks, etc. ourselves
(that the compiler can't see). So, either /GS is a waste, or it's only a
matter of time with other compilers?!

[1]
http://stackoverflow.com/questions/6607410/understanding-buffer-security-check-gs-compiler-option-in-msvc

Matt

9 years ago by Anatol Belski — view source

unread

Hi Matt,

I wonder really how much research you do :)

-----Original Message-----
From: Matt Wilmas [mailto:php_lists@realplain.com]
Sent: Monday, November 23, 2015 2:28 AM
To: internals@lists.php.net; internals-win@lists.php.net
Cc: Dmitry Stogov dmitry@zend.com; Anatol Belski
anatol.php@belski.net;
Pierre Joye pierre.php@gmail.com; Matt Tait matt.tait@gmail.com;
Nikita
Popov nikita.ppv@gmail.com
Subject: Re: [PHP-DEV] Windows (Visual Studio) compiler stuff

Hi Anatol, Dmitry, all,

Will reply about the original subject issues soon, but this is about new
stuff I
noticed the other day... Adding Matt Tait and Nikita because of PR
#1418 and comments.

Anyway, the new Control Flow Guard (/guard:cf) is causing a big slowdown
on
bench.php. :-( 14% on a Yorkfield (Q9400) and 19% on a Sandy Bridge
(Celeron
G530). Ouch. Did anyone else check the performance impact? Is this
acceptable? On any other platform...?

I'll definitely remove that from my builds (Elephpant Sanctuary, coming
soon) since it's useless on all but the latest Windows versions anyway.

But if that "feature" must remain enabled otherwise, I think we can
eliminate
most of the performance hit. As Nikita wondered about, I first wanted to
look
at the indirect calls to the opcode handlers. I tried
separating out zend_execute.c in the Makefile and added /guard:cf-
Bingo!
That restored about 98% of the speed on bench.php. It reduced the
--disable-all
NTS DLL by 13.5 KB (of the 67 KB added by full CFG).

Or could maybe change back to the old SWITCH executor? I didn't try that.

It seems like it would be a good "rule" to not use any MS stuff that isn't
done on
other compilers/platforms. :-)

/GS [1] is another that is/was starting to get annoying (function
prolog/epilog);
luckily I was able to suppress it in most cases with changes I'm making.
It's
enabled by default, of course, although I see it's
commented out in a line (old?) of confutils.js. /GS- ;-) I really
hope
there aren't places where we are not doing range checks, etc. ourselves
(that
the compiler can't see). So, either /GS is a waste, or it's only a matter
of time
with other compilers?!

[1]
http://stackoverflow.com/questions/6607410/understanding-buffer-security-
check-gs-compiler-option-in-msvc

We're unlikely to remove the security options in favor of performance. But
that's for one.

/guard:cf is documented to have possible performance impact on systems that
don't support it. However no such side effects was noticed even on win7.
There was also no bug reports in this regard. We definitely can't test any
possible HW, but it's more about OS, not HW. Is win7 your case? Then just
upgrade :)

With /GS is basically same. It's not supposed to fix the programmer
mistakes, but to add protection against exploits. Stability and
compatibility matters more than a performance trade off.

Another thing is that just one synthetic test is unlikely to reveal the big
picture. You should probably also test on some real apps, that will bring
more realistic results.

Thanks for your work.

Regards

Anatol

8 years ago by Matt Wilmas — view source

unread

Hi Anatol, all,

CFG's effect on Wordpress at the end... :-/

----- Original Message -----
From: "Anatol Belski"
Sent: Wednesday, November 25, 2015

Hi Matt,

I wonder really how much research you do :)

Not much on this... Hope there aren't major inaccuracies.

I just came across stuff while doing other things, otherwise maybe I would
have discovered this sooner!

-----Original Message-----
From: Matt Wilmas [mailto:php_lists@realplain.com]
Sent: Monday, November 23, 2015 2:28 AM
To: internals@lists.php.net; internals-win@lists.php.net
Cc: Dmitry Stogov dmitry@zend.com; Anatol Belski
anatol.php@belski.net;
Pierre Joye pierre.php@gmail.com; Matt Tait matt.tait@gmail.com;
Nikita
Popov nikita.ppv@gmail.com
Subject: Re: [PHP-DEV] Windows (Visual Studio) compiler stuff

Hi Anatol, Dmitry, all,

Will reply about the original subject issues soon, but this is about new
stuff I
noticed the other day... Adding Matt Tait and Nikita because of PR
#1418 and comments.

Anyway, the new Control Flow Guard (/guard:cf) is causing a big slowdown
on
bench.php. :-( 14% on a Yorkfield (Q9400) and 19% on a Sandy Bridge
(Celeron
G530). Ouch. Did anyone else check the performance impact? Is this
acceptable? On any other platform...?

I'll definitely remove that from my builds (Elephpant Sanctuary, coming
soon) since it's useless on all but the latest Windows versions anyway.

But if that "feature" must remain enabled otherwise, I think we can
eliminate
most of the performance hit. As Nikita wondered about, I first wanted to
look
at the indirect calls to the opcode handlers. I tried
separating out zend_execute.c in the Makefile and added /guard:cf-
Bingo!
That restored about 98% of the speed on bench.php. It reduced the
--disable-all
NTS DLL by 13.5 KB (of the 67 KB added by full CFG).

Or could maybe change back to the old SWITCH executor? I didn't try
that.

It seems like it would be a good "rule" to not use any MS stuff that
isn't
done on
other compilers/platforms. :-)

/GS [1] is another that is/was starting to get annoying (function
prolog/epilog);
luckily I was able to suppress it in most cases with changes I'm making.
It's
enabled by default, of course, although I see it's
commented out in a line (old?) of confutils.js. /GS- ;-) I really
hope
there aren't places where we are not doing range checks, etc. ourselves
(that
the compiler can't see). So, either /GS is a waste, or it's only a
matter
of time
with other compilers?!

[1]
http://stackoverflow.com/questions/6607410/understanding-buffer-security-
check-gs-compiler-option-in-msvc

We're unlikely to remove the security options in favor of performance. But
that's for one.

I didn't expect that it would be totally removed (though I will since I
consider it a useless MS "feature" when applied to PHP; again, to not use
anything that doesn't exist for other platform builds). It'd be a different
story if it didn't kill performance.

That's why I pointed out that removing it from zend_execute.c ONLY
eliminates most of the penalty. Doesn't look like many indirect calls there
to worry about besides the executor.

Are we saying that an exploit is going to modify the opcodes, etc.? Has
that ever happened (serious question)?

And since I mentioned using the old SWITCH executor, I checked and it's no
good. The specialized version is ~2.3x slower than CALL! :-O
The --without-specializer version takes forever to compile (10x longer)
and is much faster, but about ~2% slower than CALL with CFG...

/guard:cf is documented to have possible performance impact on systems
that
don't support it.

Why only on systems that don't support it? That doesn't make any sense. If
anything, I'd expect the opposite. I didn't try to check the code, but
wonder why it's not more nop-ish on unsupported systems... And I don't know
if it's just extra instructions slowing things down, or if a lookup table or
whatever for valid targets is destroying the cache.

However no such side effects was noticed even on win7.

So what did you see...? I said 19% slowdown on bench.php, Sandy Bridge, Win
7 x64, 32-bit. Have since checked 64-bit build and that's 20%.

There was also no bug reports in this regard.

Bug reports against what...? Probably nobody was looking, and first it was
VS 2012 before 2015 and then enabling CFG. I guess I could file one now
since testing Wordpress?

We definitely can't test any
possible HW, but it's more about OS, not HW.

I'd wager the opposite... It's affecting CPU stuff, after all.

Is win7 your case? Then just upgrade :)

Yeah, on the one system. The Yorkfield is Windows XP gasp. Definitely
won't just "upgrade" any system unless desired for reasons overall. And as
long as I'm not satisfied with desktop Linux, Hackintosh, ....

Anyway, I need XP, and there's really no legitimate reason to not support
it (only FUD/lies, sorry ;-))... I was just waiting on MS bugs [1][2] to
allow VC14 to be used, which 2015 Update 1 fixes! :-O Even full-of-XP-lies
Microsoft still supports the VC runtime, at least.

[1]
https://social.msdn.microsoft.com/Forums/vstudio/en-US/52b0c797-6fa7-4933-8bb2-fe90e8764e27/visual-c-2015-express-stat-not-working-on-windows-xp?forum=vcgeneral
[2]
https://connect.microsoft.com/VisualStudio/feedback/details/1557168/wstat64-returns-1-on-xp-always

To give another idea of how much CFG hurts: it takes bench.php performance
back to VS 2008 level (works with just a few more changes). 7 years of
compiler improvement just eliminated!

With /GS is basically same. It's not supposed to fix the programmer
mistakes, but to add protection against exploits. Stability and
compatibility matters more than a performance trade off.

I didn't check anything (performance) with /GS- just complaining about the
extra code it adds to functions. :-) (Again useless in almost all cases
since we already have checks...)

Another thing is that just one synthetic test is unlikely to reveal the
big
picture. You should probably also test on some real apps, that will bring
more realistic results.

Wordpress 4.3.1 results (MySQL 5.5, no query cache):

~5% slower on Win 7, 32-bit, Sandy Bridge
6~7% slower on Win XP, Yorkfield

(Had to fix Wordpress to allow persistent connections ("p:localhost"), since
Windows connections (TCP and pipes) are EXTREMELY slow since some years
(5-10) ago, for some reason. :-( Dunno if it's after mysqlnd or what, never
really investigated.)

That's a pretty big deal, considering the work that has been done for 1-2%
speedups ("big enough" or "worthwhile" improvements).

Obviously anything that uses opcodes is affected (read: everything).

Thanks for your work.

Regards

Anatol

Matt