ARM Skia NEON patches - 41 - arm64: SkXfermode::xfer32

src/opts/SkXfermode_opts_arm_neon.cpp

 #include "SkXfermode.h"
 #include "SkXfermode_proccoeff.h"
 #include "SkColorPriv.h"
 
 #include <arm_neon.h>
 #include "SkColor_opts_neon.h"
 #include "SkXfermode_opts_arm_neon.h"
 
 #define SkAlphaMulAlpha(a, b)   SkMulDiv255Round(a, b)
 
@@ skipping 730 matching lines @@
 
 typedef uint8x8x4_t (*SkXfermodeProcSIMD)(uint8x8x4_t src, uint8x8x4_t dst);
 
 extern SkXfermodeProcSIMD gNEONXfermodeProcs[];
 
 SkNEONProcCoeffXfermode::SkNEONProcCoeffXfermode(SkReadBuffer& buffer)
         : INHERITED(buffer) {
     fProcSIMD = reinterpret_cast<void*>(gNEONXfermodeProcs[this->getMode()]);
 }
 
-void SkNEONProcCoeffXfermode::xfer32(SkPMColor dst[], const SkPMColor src[],
-                                     int count, const SkAlpha aa[]) const {
+void SkNEONProcCoeffXfermode::xfer32(SkPMColor* SK_RESTRICT dst,
+                                     const SkPMColor* SK_RESTRICT src, int count,
+                                     const SkAlpha* SK_RESTRICT aa) const {
     SkASSERT(dst && src && count >= 0);
 
     SkXfermodeProc proc = this->getProc();
     SkXfermodeProcSIMD procSIMD = reinterpret_cast<SkXfermodeProcSIMD>(fProcSIMD);
     SkASSERT(procSIMD != NULL);
 
     if (NULL == aa) {
         // Unrolled NEON code
         while (count >= 8) {

[mtklein, 2014/06/30 13:44:35]

Do you think you could tack on some note like this for me for next time?

// We'd like to just do this:
//   vst4_8(dst, procSIMD(vld4_8(src), vld4_8(dst)));
//   src += 8;
//   dst += 8;
// but that tends to generate miserable code.  Here are a bunch of faster
// workarounds for different architectures and compilers.

[kevin.petit, 2014/06/30 14:04:39]

Done.

+
+#ifdef SK_CPU_ARM32
             uint8x8x4_t vsrc, vdst, vres;
-
-#ifdef SK_CPU_ARM64
-            vsrc = vld4_u8((uint8_t*)src);
-            vdst = vld4_u8((uint8_t*)dst);
-#else
 #if (__GNUC__ > 4) || ((__GNUC__ == 4) && (__GNUC_MINOR__ > 6))
             asm volatile (
                 "vld4.u8    %h[vsrc], [%[src]]!  \t\n"
                 "vld4.u8    %h[vdst], [%[dst]]   \t\n"
                 : [vsrc] "=w" (vsrc), [vdst] "=w" (vdst), [src] "+&r" (src)
                 : [dst] "r" (dst)
                 :
             );
 #else
             register uint8x8_t d0 asm("d0");
@@ skipping 12 matching lines @@
                   "=w" (d4), "=w" (d5), "=w" (d6), "=w" (d7),
                   [src] "+&r" (src)
                 : [dst] "r" (dst)
                 :
             );
             vsrc.val[0] = d0; vdst.val[0] = d4;
             vsrc.val[1] = d1; vdst.val[1] = d5;
             vsrc.val[2] = d2; vdst.val[2] = d6;
             vsrc.val[3] = d3; vdst.val[3] = d7;
 #endif
-#endif // #ifdef SK_CPU_ARM64
 
             vres = procSIMD(vsrc, vdst);
 
             vst4_u8((uint8_t*)dst, vres);
 
+            dst += 8;
+
+#else // #ifdef SK_CPU_ARM32
+
+            asm volatile (

[mtklein, 2014/06/30 13:28:03]

Can you take me through this and remind my why it can't be intrinsics
everywhere?  Seems like we only want something super simple:

vst4_8(dst, procSIMD(vld4_8(src), vld4_8(dst)));
src += 8;
dst += 8;

is that wrong, unsupported, or just not fast enough?  Are we getting the win
here from the post-increments?  Is there really no way to get GCC to generate
post-increments?

[kevin.petit, 2014/06/30 13:38:05]

Correct.
It's _really_ slower than what it should be. Try to build with the intrinsics
and have a look at the assembly if you're feeling brave enough. Various
toolchains are broken in various ways and all that assembly is only here to work
around the issues.
We do gain from the post-increments but in a lot of cases, aka when it's not
broken, gcc is able to use them.

+                "ld4    {v0.8b - v3.8b}, [%[src]], #32 \t\n"
+                "ld4    {v4.8b - v7.8b}, [%[dst]]      \t\n"
+                "blr    %[proc]                        \t\n"
+                "st4    {v0.8b - v3.8b}, [%[dst]], #32 \t\n"
+                : [src] "+&r" (src), [dst] "+&r" (dst)
+                : [proc] "r" (procSIMD)
+                : "cc", "memory",
+                  /* We don't know what proc is going to clobber so we must
+                   * add everything that is not callee-saved.
+                   */
+                  "x0", "x1", "x2", "x3", "x4", "x5", "x6", "x7", "x8", "x9",
+                  "x10", "x11", "x12", "x13", "x14", "x15", "x16", "x17", "x18",
+                  "v0", "v1", "v2", "v3", "v4", "v5", "v6", "v7", "v16", "v17",
+                  "v18", "v19", "v20", "v21", "v22", "v23", "v24", "v25", "v26",
+                  "v27", "v28", "v29", "v30", "v31"
+            );
+
+#endif // #ifdef SK_CPU_ARM32
+
             count -= 8;
-            dst += 8;
-#ifdef SK_CPU_ARM64
-            src += 8;
-#endif
         }
         // Leftovers
         for (int i = 0; i < count; i++) {
             dst[i] = proc(src[i], dst[i]);
         }
     } else {
         for (int i = count - 1; i >= 0; --i) {
             unsigned a = aa[i];
             if (0 != a) {
                 SkPMColor dstC = dst[i];
@@ skipping 175 matching lines @@
 
     if (procSIMD != NULL) {
         return SkNEW_ARGS(SkNEONProcCoeffXfermode, (rec, mode, procSIMD));
     }
     return NULL;
 }
 
 SkXfermodeProc SkPlatformXfermodeProcFactory_impl_neon(SkXfermode::Mode mode) {
     return gNEONXfermodeProcs1[mode];
 }