Hi Ard,
Actually with your patch the fast_increment_function() is going to be
compile into something like this
incl (%ecx)
seto %al
test %al,%al
jz .FLOAT
.END:
...
.FLOAT:
movl $0x0, (%ecx)
movl $0x41e00000, 0x4(%ecx)
movb $0x2,0xc(%ecx)
jmp . END
while before the patch it would
incl (%ecx)
jno .END
.FLOATL
movl $0x0, (%ecx)
movl $0x41e00000, 0x4(%ecx)
movb $0x2,0xc(%ecx)
.END:
...
So the only advantage of your code is eliminated static branch
misprediction in cost of two additional CPU instructions.
However CPU branch predictor should make this advantage unimportant.
Thanks. Dmitry.
On Fri, Jan 18, 2013 at 10:08 PM, Ard Biesheuvel
<ard.biesheu...@linaro.org>wrote:
> Hello,
>
> Again, apologies for prematurely declaring someone else's code 'crap'.
> There are no bugs in the inline x86 assembler in Zend/zend_operators.h, as
> far as I can tell, only two kinds of issues that I still think should be
> addressed.
>
> First of all, from a maintenance pov, having field offsets (like the
> offset of zval.type) and constants (like $0x2 for IS_DOUBLE) hard coded
> inside the instructions is a bad idea.
>
> The other issue is the branching and the floating point instructions. The
> inline assembler addresses the common case, but also adds a bunch of
> instructions that address the corner case, and some branches to jump over
> them. As I indicated in my previous email, branching is relatively costly
> on a modern CPU with deep pipelines and having a bunch of FPU instructions
> in there that hardly ever get executed doesn't help either.
>
> The primary reason for having inline assembler at all is the ability to
> detect overflow. This mainly applies to multiplication, as in that case,
> detecting overflow in C code is much harder compared to reading a condition
> flag in the CPU (hence the various accelerated implementations in
> zend_signed_multiply.h). However, detecting overflow in
> addition/subtraction implemented in C is much easier, as the code in
> zend_operators.h proves: just a matter of checking the sign bits, or doing
> a simple compare with LONG_MIN/LONG_MAX.
>
> Therefore, I would be interested in finding out which benchmark was used
> to make the case for having these accelerated implementations in the first
> place. The differences in performance between various implementations are
> very small in the tests I have done.
>
> As for the code style/maintainability, I propose to apply the attached
> patch. The performance is on par, as far as I can tell, but it is arguably
> better code. I will also hook in the ARM versions once I manage to prove
> that the performance is affected favourably by them.
>
> Regards,
> Ard.
>
>
>
> Before
> -------
>
> $ time php -r 'for ($i = 0; $i < 0x7fffffff; $i++);'
>
> real 0m56.910s
> user 0m56.876s
> sys 0m0.008s
>
>
> $ time php -r 'for ($i = 0x7fffffff; $i >= 0; $i--);'
>
> real 1m34.576s
> user 1m34.518s
> sys 0m0.020s
>
>
> $ time php -r 'for ($i = 0; $i < 0x7fffffff; $i += 3);'
>
> real 0m21.494s
> user 0m21.473s
> sys 0m0.008s
>
>
> $ time php -r 'for ($i = 0x7fffffff; $i >= 0; $i -= 3);'
>
> real 0m19.879s
> user 0m19.865s
> sys 0m0.004s
>
>
> After
> -----
>
> $ time php -r 'for ($i = 0; $i < 0x7fffffff; $i++);'
>
> real 0m56.687s
> user 0m56.656s
> sys 0m0.004s
>
>
> $ time php -r 'for ($i = 0x7fffffff; $i >= 0; $i--);'
>
> real 1m28.124s
> user 1m28.082s
> sys 0m0.004s
>
>
> $ time php -r 'for ($i = 0; $i < 0x7fffffff; $i += 3);'
>
> real 0m20.561s
> user 0m20.545s
> sys 0m0.004s
>
>
> $ time php -r 'for ($i = 0x7fffffff; $i >= 0; $i -= 3);'
>
> real 0m20.524s
> user 0m20.509s
> sys 0m0.004s
>
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
