[Pixman] [PATCH] Add support for aarch64 neon optimization

Siarhei Siamashka siarhei.siamashka at gmail.com
Thu Apr 7 08:14:27 UTC 2016


On Tue, 5 Apr 2016 19:28:19 +0900
Mizuki Asakura <ed6e117f at gmail.com> wrote:

> > It looks like you have used an automated process to convert the AArch32
> > NEON code to AArch64. Will you be able to repeat that process for other
> > code, or at least assist others to repeat your steps?  
> 
> Sorry, but I've wrote before, all of the patch were converted by hand.
> "converter script" didn't work correctly.
> # But the script was very helpful for me to understand the difference
> # between aarch32 and aarch64 :)
> 
> 
> > The reason I ask is that I have a large number of outstanding patches to
> > the ARM NEON support.  
> 
> Hmm...
> How should we proceed the implementation ?

Unless there are some objections, I would prefer to see your patch
just cleaned up a bit and pushed to the pixman repository. And then
we could have the next pixman version tagged soon.

> I've seen a comment that current (and I've based) pixman-arm-neon-asm*.S
> were optimized on older Cortex-A8. And, your new patches seem to be
> working well on latest Cortex chips.

Different ARM processors behave in a slightly different way, but that's
not the main point.

As far as the assembly code is concerned, Ben's patches are introducing
a new way of doing bilinear scaling. Implementing a separable algorithm,
previously implemented by Søren for x86 using SSSE3 instructions:

    https://lists.freedesktop.org/archives/pixman/2013-September/002900.html

There are both advantages and drawbacks to this scaling method, but it
is clearly useful if used wisely.

> If so, we should first apply your latest patch to the master, and then,
> someone (or I ?) do the conversion to aarch64 again. It would be good both
> aarch32 and aarch64 worlds.

There is no point suddenly turning the ARM assembly code into a moving
target right now. The Ben's code is not the first and hopefully not the
last change to the 32-bit ARM assembly source files in pixman history.

We will have to find a way to keep this stuff maintainable. That's why
I still think that a fully automated conversion and code sharing
between AArch64 and AArch32 is possible. But it can be introduced at a
bit later date. We should prioritize getting the AArch64 optimized
pixman release to the users as soon as possible.

> # FYI: I've spent 1 week to convert all of the code,
> # and 2 weeks to pass all tests.

Thanks. That was a good work. I guess, now we only need a few more
days, maybe a week maximum to clean this patch a bit. For example,
I spent roughly one evening on doing the most part of these cleanups:

   https://cgit.freedesktop.org/~siamashka/pixman/commit/?h=20160401-arm64-review&id=2f8c71416232bb714bb2420440333496b36fbfae
   https://cgit.freedesktop.org/~siamashka/pixman/commit/?h=20160401-arm64-review&id=76ad1ba645489e6f987da72a7e7f9fa3ef72141c

And while 2f8c71416232bb714bb2420440333496b36fbfae may look like
some major code changes, in fact I was changing it to reduce the
differences between the 32-bit and 64-bit code, looking at the
sources side by side.

I don't think that splitting the patches is necessary (other than
handling pixman-arm-neon-asm-bilinear.S separately). Because this
is not a usual patch review process (looking at the incremental
code changes), but more like the validation of the code conversion
quality. Either way, I'll be looking at the 32-bit and the 64-bit
code side by side when reviewing it. And splitting the patches may
make this more difficult.

And as we seem to be running in circles in this discussion (mostly
the same questions are getting repeated multiple times), I have also
created the following wiki page with the hope to keep the relevant
information more structured:

    https://pixman.miraheze.org/wiki/AArch64_Support

I'll try to add more information about the fully automated conversion
to the wiki later.

-- 
Best regards,
Siarhei Siamashka


More information about the Pixman mailing list