[Pixman] [PATCH 1/4] pixman-fast-path: Add over_n_8888 fast path (disabled)
ppaalanen at gmail.com
Thu Aug 27 02:59:52 PDT 2015
On Wed, 26 Aug 2015 12:06:59 +0100
"Ben Avison" <bavison at riscosopen.org> wrote:
> On Wed, 26 Aug 2015 09:46:49 +0100, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> > It's clearly controversial to add C fast paths, because it affects all
> > targets that don't have an asm fast path for the same, and we cannot
> > tell by just review whether it is going to be for better or (much)
> > worse.
> Yes, it's always going to be a risk changing the cross-platform
> functions. Few developers are going to be able to test all the supported
> platforms, so we're always going to need help checking performance.
> Should that be a reason not to even try C fast paths though?
This is very much IMHO, but (to not even try):
- yes, if one is going to have a platform-specific fast path anyway
- yes, if one can reasonably write a platform-specific fast path, that
is, you are working on just one or maybe two platforms
- ??, if it's something new that rarely any platform accelerates
If Oded hadn't benchmarked this, we would likely have landed this C
fast path. Then no-one would have realized it's actually slower than
the generic path afterwards.
Whether this is an isolated case or not, I don't know. But since we
tend to be more safe than sorry with pixman, I would err on the side of
not writing C fast paths. Hmm, assuming you can beat the compiler with
Or, we could say we want N amount of checking on different platforms
before we can land a new C fast path, but I suspect that is as good as
just dropping it, considering how much reviews we get on patches.
It would be *really* nice if we could somehow use a benchmark mode
where we could run an operation with every possible implementation and
compare them. I wonder, can we already do that with PIXMAN_DISABLE?
I see the code recognizes these names for PIXMAN_DISABLE:
pixman/pixman-arm.c:210: if (!_pixman_disabled ("arm-simd") && have_feature (ARM_V6))
pixman/pixman-arm.c:215: if (!_pixman_disabled ("arm-iwmmxt") && have_feature (ARM_IWMMXT))
pixman/pixman-arm.c:220: if (!_pixman_disabled ("arm-neon") && have_feature (ARM_NEON))
pixman/pixman-implementation.c:390: if (!_pixman_disabled ("fast"))
pixman/pixman-mips.c:73: if (!_pixman_disabled ("loongson-mmi") && have_feature ("Loongson"))
pixman/pixman-mips.c:78: if (!_pixman_disabled ("mips-dspr2"))
pixman/pixman-ppc.c:150: if (!_pixman_disabled ("vmx") && pixman_have_vmx ())
pixman/pixman-x86.c:233: if (!_pixman_disabled ("mmx") && have_feature (MMX_BITS))
pixman/pixman-x86.c:238: if (!_pixman_disabled ("sse2") && have_feature (SSE2_BITS))
pixman/pixman-x86.c:243: if (!_pixman_disabled ("ssse3") && have_feature (SSSE3_BITS))
Would that be everything we need to control to be able to test every
possible path on a platform?
This could be the way to test whether Pixman is choosing the optimal
implementation of what it already has. Some heavy scripting around
lowlevel-blt-bench could produce a nice table of results.
Just an idea. Of course it doesn't remove the need to actually try
things on the various machines.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 811 bytes
Desc: OpenPGP digital signature
More information about the Pixman