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

src/opts/SkXfermode_opts_arm_neon.cpp

Index: src/opts/SkXfermode_opts_arm_neon.cpp
diff --git a/src/opts/SkXfermode_opts_arm_neon.cpp b/src/opts/SkXfermode_opts_arm_neon.cpp
index 70e92af66bc548f9f42c0ecc70b8f8513794d096..eac7e73815be1ac6def35958b8295096216d67fd 100644
--- a/src/opts/SkXfermode_opts_arm_neon.cpp
+++ b/src/opts/SkXfermode_opts_arm_neon.cpp
@@ -748,8 +748,9 @@ SkNEONProcCoeffXfermode::SkNEONProcCoeffXfermode(SkReadBuffer& 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();
@@ -759,12 +760,9 @@ void SkNEONProcCoeffXfermode::xfer32(SkPMColor dst[], const SkPMColor src[],
     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.

-            uint8x8x4_t vsrc, vdst, vres;
 
-#ifdef SK_CPU_ARM64
-            vsrc = vld4_u8((uint8_t*)src);
-            vdst = vld4_u8((uint8_t*)dst);
-#else
+#ifdef SK_CPU_ARM32
+            uint8x8x4_t vsrc, vdst, vres;
 #if (__GNUC__ > 4) || ((__GNUC__ == 4) && (__GNUC_MINOR__ > 6))
             asm volatile (
                 "vld4.u8    %h[vsrc], [%[src]]!  \t\n"
@@ -797,17 +795,36 @@ void SkNEONProcCoeffXfermode::xfer32(SkPMColor dst[], const SkPMColor src[],
             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);
 
-            count -= 8;
             dst += 8;
-#ifdef SK_CPU_ARM64
-            src += 8;
-#endif
+
+#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;
         }
         // Leftovers
         for (int i = 0; i < count; i++) {