<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Jul 18, 2013 at 2:25 AM,  <span dir="ltr"><<a href="mailto:gopu@multicorewareinc.com" target="_blank">gopu@multicorewareinc.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"># HG changeset patch<br>
# User ggopu<br>
# Date 1374132302 -19800<br>
# Node ID d93bf22889f8a58c3b3a03733c8e031ffe192fc3<br>
# Parent  f813f110d69a1a6650e813dd4e612216982a0264<br>
Primitive: Performance Primitives for Pixel add Clip - TcomYuv and TshortYuv<br>
<br>
diff -r f813f110d69a -r d93bf22889f8 source/common/pixel.cpp<br>
--- a/source/common/pixel.cpp   Thu Jul 18 02:10:37 2013 -0500<br>
+++ b/source/common/pixel.cpp   Thu Jul 18 12:55:02 2013 +0530<br>
@@ -535,6 +535,35 @@<br>
     }<br>
 }<br>
<br>
+void pixeladd_ss_c(int bx, int by, short *a, intptr_t dstride, short *b0, short *b1, intptr_t sstride0, intptr_t sstride1)<br>
+{<br>
+    for (int y = 0; y < by; y++)<br>
+    {<br>
+        for (int x = 0; x < bx; x++)<br>
+        {<br>
+            a[x] = (short)ClipY(b0[x] + b1[x]);<br>
+        }<br>
+<br>
+        b0 += sstride0;<br>
+        b1 += sstride1;<br>
+        a += dstride;<br>
+    }<br>
+}<br>
+<br>
+void pixeladd_pp_c(int bx, int by, pixel *a, intptr_t dstride, pixel *b0, pixel *b1, intptr_t sstride0, intptr_t sstride1)<br>
+{<br>
+    for (int y = 0; y < by; y++)<br>
+    {<br>
+        for (int x = 0; x < bx; x++)<br>
+        {<br>
+            a[x] = (pixel)ClipY(b0[x] + b1[x]);<br>
+        }<br>
+<br>
+        b0 += sstride0;<br>
+        b1 += sstride1;<br>
+        a += dstride;<br>
+    }<br>
+}<br>
 }  // end anonymous namespace<br>
<br>
 namespace x265 {<br>
@@ -738,5 +767,7 @@<br>
     p.weightpUni = weightUnidir;<br>
<br>
     p.pixelsub_sp = pixelsub_sp_c;<br>
+    p.pixeladd_pp = pixeladd_pp_c;<br>
+    p.pixeladd_ss = pixeladd_ss_c;<br>
 }<br>
 }<br>
diff -r f813f110d69a -r d93bf22889f8 source/common/primitives.h<br>
--- a/source/common/primitives.h        Thu Jul 18 02:10:37 2013 -0500<br>
+++ b/source/common/primitives.h        Thu Jul 18 12:55:02 2013 +0530<br>
@@ -192,7 +192,9 @@<br>
 typedef void (*blockcpy_sp_t)(int bx, int by, short *dst, intptr_t dstride, pixel *src, intptr_t sstride); // dst is aligned<br>
 typedef void (*blockcpy_ps_t)(int bx, int by, pixel *dst, intptr_t dstride, short *src, intptr_t sstride); // dst is aligned<br>
 typedef void (*blockcpy_sc_t)(int bx, int by, short *dst, intptr_t dstride, uint8_t *src, intptr_t sstride); // dst is aligned<br>
-typedef void (*pixelsub_sp_t)(int bx, int by, short *dst, intptr_t dstride, pixel *src0, pixel *src1, intptr_t sstride0, intptr_t sstride1);<br>
+typedef void (*pixelsub_sp_t)(int bx, int by, short *dst, intptr_t dstride, pixel *src0, pixel *src1, intptr_t sstride0, intptr_t sstride1);<br>
+typedef void (*pixeladd_ss_t)(int bx, int by, short *dst, intptr_t dstride, short *src0, short *src1, intptr_t sstride0, intptr_t sstride1);<br>
+typedef void (*pixeladd_pp_t)(int bx, int by, pixel *dst, intptr_t dstride, pixel *src0, pixel *src1, intptr_t sstride0, intptr_t sstride1);<br>
<br>
 typedef void (*intra_dc_t)(pixel* above, pixel* left, pixel* dst, intptr_t dstStride, int width, int bFilter);<br>
 typedef void (*intra_planar_t)(pixel* src, intptr_t srcStride, pixel* dst, intptr_t dstStride, int width);<br>
@@ -275,6 +277,8 @@<br>
<br>
     weightpUni_t    weightpUni;<br>
     pixelsub_sp_t   pixelsub_sp;<br>
+    pixeladd_ss_t   pixeladd_ss;<br>
+    pixeladd_pp_t   pixeladd_pp;<br>
<br>
     filterVwghtd_t  filterVwghtd;<br>
     filterHwghtd_t  filterHwghtd;<br></blockquote><div><br></div><div><br></div><div>everything up to this point looks good</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

diff -r f813f110d69a -r d93bf22889f8 source/common/vec/blockcopy.inc<br>
--- a/source/common/vec/blockcopy.inc   Thu Jul 18 02:10:37 2013 -0500<br>
+++ b/source/common/vec/blockcopy.inc   Thu Jul 18 12:55:02 2013 +0530<br>
@@ -292,6 +292,162 @@<br>
     }<br>
 }<br>
<br>
+void pixeladd_ss(int bx, int by, short *dst, intptr_t dstride, short *src0, short *src1, intptr_t sstride0, intptr_t sstride1)<br>
+{<br>
+    size_t aligncheck = (size_t)dst | (size_t)src0 | bx | sstride0 | dstride;<br></blockquote><div><br></div><div>This is still not checking bx correctly.  It needs to be checked separately from the pointers and strides</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+#if INSTRSET >= 8 && 0<br>
+    if (!(aligncheck & 31))<br>
+    {<br>
+        // fast path, multiples of 32 pixel wide blocks<br>
+        // fast path, multiples of 16 pixel wide blocks<br></blockquote><div><br></div><div>one of these comments is wrong</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

+        for (int y = 0; y < by; y++)<br>
+        {<br>
+            for (int x = 0; x < bx; x += 32)<br>
+            {<br>
+                Vec32s vecsrc0, vecsrc1, vecsum;<br>
+                Vec32s zero(0), maxval((1 << X265_DEPTH) - 1); // Currently g_bitDepthY = 8 and g_bitDepthC = 8<br></blockquote><div><br></div><div>The comment above is wrong.  zero and maxval should be removed from out of the loop</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+                vecsrc0.load_a(src0 + x);<br>
+                vecsrc1.load_a(src1 + x);<br>
+<br>
+                vecsum = vecsrc0 + vecsrc1;<br>
+                vecsum = max(vecsum, zero);<br>
+                vecsum = min(vecsum, maxval);<br>
+<br>
+                vecsum.store(dst + x);<br>
+            }<br>
+<br>
+            src0 += sstride0;<br>
+            src1 += sstride1;<br>
+            dst += dstride;<br>
+        }<br>
+    }<br>
+    else<br>
+#endif /* if INSTRSET >= 8 && 0 */<br>
+    if (!(aligncheck & 15))<br>
+    {<br>
+        // fast path, multiples of 16 pixel wide blocks<br>
+        for (int y = 0; y < by; y++)<br>
+        {<br>
+            for (int x = 0; x < bx; x += 8)<br>
+            {<br>
+                Vec8s vecsrc0, vecsrc1, vecsum;<br>
+                Vec8s zero(0), maxval((1 << X265_DEPTH) - 1); // Currently g_bitDepthY = 8 and g_bitDepthC = 8<br></blockquote><div><br></div><div>same wrong comment</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

+                vecsrc0.load_a(src0 + x);<br>
+                vecsrc1.load_a(src1 + x);<br>
+<br>
+                vecsum = add_saturated(vecsrc0, vecsrc1);<br>
+                vecsum = max(vecsum, zero);<br>
+                vecsum = min(vecsum, maxval);<br>
+<br>
+                vecsum.store(dst + x);<br>
+            }<br>
+<br>
+            src0 += sstride0;<br>
+            src1 += sstride1;<br>
+            dst += dstride;<br>
+        }<br>
+    }<br>
+    else<br>
+    {<br></blockquote><div><br></div><div>there needs to be another else clause that uses SIMD for the copies but uses unaligned loads, if bx is a multiple of 8</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

+        int tmp;<br>
+        int max = (1 << X265_DEPTH) - 1;<br>
+        // slow path, irregular memory alignments or sizes<br>
+        for (int y = 0; y < by; y++)<br>
+        {<br>
+            for (int x = 0; x < bx; x++)<br>
+            {<br>
+                tmp = src0[x] + src1[x];<br>
+                tmp = tmp < 0 ? 0 : tmp;<br>
+                tmp = tmp > max ? max : tmp;<br>
+                dst[x] = (short)tmp;<br>
+            }<br>
+<br>
+            src0 += sstride0;<br>
+            src1 += sstride1;<br>
+            dst += dstride;<br>
+        }<br>
+    }<br>
+}<br></blockquote><div><br></div><div>This function should only be compiled for HIGH_BIT_DEPTH, else it is just wrong</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

+void pixeladd_pp(int bx, int by, pixel *dst, intptr_t dstride, pixel *src0, pixel *src1, intptr_t sstride0, intptr_t sstride1)<br>
+{<br>
+    size_t aligncheck = (size_t)dst | (size_t)src0 | bx | sstride0 | dstride;<br>
+<br>
+#if INSTRSET >= 8 && 0<br>
+    if (!(aligncheck & 31))<br>
+    {<br>
+        // fast path, multiples of 32 pixel wide blocks<br>
+        // fast path, multiples of 16 pixel wide blocks<br></blockquote><div><br></div><div>One of these comments is wrong</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

+        for (int y = 0; y < by; y++)<br>
+        {<br>
+            for (int x = 0; x < bx; x += 32)<br>
+            {<br>
+                Vec32s vecsrc0, vecsrc1, vecsum;<br>
+                Vec32s zero(0), maxval((1 << X265_DEPTH) - 1); // Currently g_bitDepthY = 8 and g_bitDepthC = 8<br></blockquote><div><br></div><div>It's copying short values up here</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

+                vecsrc0.load_a(src0 + x);<br>
+                vecsrc1.load_a(src1 + x);<br>
+<br>
+                vecsum = vecsrc0 + vecsrc1;<br>
+                vecsum = max(vecsum, zero);<br>
+                vecsum = min(vecsum, maxval);<br>
+<br>
+                vecsum.store(dst + x);<br>
+            }<br>
+<br>
+            src0 += sstride0;<br>
+            src1 += sstride1;<br>
+            dst += dstride;<br>
+        }<br>
+    }<br>
+    else<br>
+#endif /* if INSTRSET >= 8 && 0 */<br>
+    if (!(aligncheck & 15))<br>
+    {<br>
+        // fast path, multiples of 16 pixel wide blocks<br>
+        for (int y = 0; y < by; y++)<br>
+        {<br>
+            for (int x = 0; x < bx; x += 16)<br>
+            {<br>
+                Vec16uc vecsrc0, vecsrc1, vecsum;<br>
+                Vec16uc zero(0), maxval((1 << X265_DEPTH) - 1); // Currently g_bitDepthY = 8 and g_bitDepthC = 8<br>
+                vecsrc0.load_a(src0 + x);<br>
+                vecsrc1.load_a(src1 + x);<br></blockquote><div><br></div><div>And copying char values down here</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

+                vecsum = add_saturated(vecsrc0, vecsrc1);<br>
+                vecsum = max(vecsum, zero);<br>
+                vecsum = min(vecsum, maxval);<br>
+<br>
+                vecsum.store(dst + x);<br>
+            }<br>
+<br>
+            src0 += sstride0;<br>
+            src1 += sstride1;<br>
+            dst += dstride;<br>
+        }<br>
+    }<br>
+    else<br></blockquote><div><br></div><div>ditto for an additional else if () clause here for unaligned pointers or strides</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

+    {<br>
+        int tmp;<br>
+        int max = (1 << X265_DEPTH) - 1;<br>
+        // slow path, irregular memory alignments or sizes<br>
+        for (int y = 0; y < by; y++)<br>
+        {<br>
+            for (int x = 0; x < bx; x++)<br>
+            {<br>
+                tmp = src0[x] + src1[x];<br>
+                tmp = tmp < 0 ? 0 : tmp;<br>
+                tmp = tmp > max ? max : tmp;<br>
+                dst[x] = (pixel)tmp;<br>
+            }<br>
+<br>
+            src0 += sstride0;<br>
+            src1 += sstride1;<br>
+            dst += dstride;<br>
+        }<br>
+    }<br>
+}<br>
+<br>
 void Setup_Vec_BlockCopyPrimitives(EncoderPrimitives &p)<br>
 {<br>
 #if HIGH_BIT_DEPTH<br>
@@ -300,11 +456,15 @@<br>
     p.blockcpy_ps = (x265::blockcpy_ps_t)blockcopy_p_p;<br>
     p.blockcpy_sp = (x265::blockcpy_sp_t)blockcopy_p_p;<br>
     p.blockcpy_sc = (x265::blockcpy_sc_t)blockcopy_s_p;<br>
+    p.pixeladd_pp = (x265::pixeladd_pp_t)pixeladd_ss;<br></blockquote><div><br></div><div>If TShortYuv was changed to use UShort buffers, this typecast would be unnecessary.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

+    p.pixeladd_ss = pixeladd_ss;<br>
 #else<br>
     p.blockcpy_pp = blockcopy_p_p;<br>
     p.blockcpy_ps = blockcopy_p_s;<br>
     p.blockcpy_sp = blockcopy_s_p;<br>
     p.blockcpy_sc = blockcopy_s_p;<br>
     p.pixelsub_sp = pixelsub_sp;<br>
-#endif<br>
+    p.pixeladd_ss = pixeladd_ss;<br>
+    p.pixeladd_pp = pixeladd_pp;<br>
+#endif /* if HIGH_BIT_DEPTH */<br>
 }<br>
diff -r f813f110d69a -r d93bf22889f8 source/common/vec/vecprimitives.inc<br>
--- a/source/common/vec/vecprimitives.inc       Thu Jul 18 02:10:37 2013 -0500<br>
+++ b/source/common/vec/vecprimitives.inc       Thu Jul 18 12:55:02 2013 +0530<br>
@@ -28,6 +28,9 @@<br>
 #include "utils.h"<br>
 #include <string.h><br>
<br>
+#include "TLibCommon\TComRom.h"<br>
+#include "TLibCommon\TypeDef.h"<br></blockquote><div><br></div><div>You must use forward slashes in include statements, else this will not compile on Linux</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

+<br>
 using namespace x265;<br>
<br>
 namespace {<br>
diff -r f813f110d69a -r d93bf22889f8 source/test/pixelharness.cpp<br>
--- a/source/test/pixelharness.cpp      Thu Jul 18 02:10:37 2013 -0500<br>
+++ b/source/test/pixelharness.cpp      Thu Jul 18 12:55:02 2013 +0530<br>
@@ -376,6 +376,52 @@<br>
     return true;<br>
 }<br>
<br>
+bool PixelHarness::check_pixeladd_ss(x265::pixeladd_ss_t ref, x265::pixeladd_ss_t opt)<br>
+{<br>
+    ALIGN_VAR_16(short, ref_dest[64 * 64]);<br>
+    ALIGN_VAR_16(short, opt_dest[64 * 64]);<br>
+    int bx = 64;<br>
+    int by = 64;<br>
+    int j = 0;<br>
+    for (int i = 0; i <= 100; i++)<br>
+    {<br>
+        opt(bx, by, opt_dest, 64, (short*)pbuf2 + j, (short*)pbuf1 + j, 128, 128);<br>
+        ref(bx, by, ref_dest, 64, (short*)pbuf2 + j, (short*)pbuf1 + j, 128, 128);<br>
+<br>
+        if (memcmp(ref_dest, opt_dest, 64 * 64 * sizeof(short)))<br>
+            return false;<br>
+<br>
+        j += 4;<br>
+        bx = 4 * ((rand() & 15) + 1);<br>
+        by = 4 * ((rand() & 15) + 1);<br>
+    }<br>
+<br>
+    return true;<br>
+}<br>
+<br>
+bool PixelHarness::check_pixeladd_pp(x265::pixeladd_pp_t ref, x265::pixeladd_pp_t opt)<br>
+{<br>
+    ALIGN_VAR_16(pixel, ref_dest[64 * 64]);<br>
+    ALIGN_VAR_16(pixel, opt_dest[64 * 64]);<br>
+    int bx = 64;<br>
+    int by = 64;<br>
+    int j = 0;<br>
+    for (int i = 0; i <= 100; i++)<br>
+    {<br>
+        opt(bx, by, opt_dest, 64, pbuf2 + j, pbuf1 + j, 128, 128);<br>
+        ref(bx, by, ref_dest, 64, pbuf2 + j, pbuf1 + j, 128, 128);<br>
+<br>
+        if (memcmp(ref_dest, opt_dest, 64 * 64 * sizeof(pixel)))<br>
+            return false;<br>
+<br>
+        j += 4;<br>
+        bx = 4 * ((rand() & 15) + 1);<br>
+        by = 4 * ((rand() & 15) + 1);<br>
+    }<br>
+<br>
+    return true;<br>
+}<br>
+<br>
 bool PixelHarness::testCorrectness(const EncoderPrimitives& ref, const EncoderPrimitives& opt)<br>
 {<br>
     for (uint16_t curpar = 0; curpar < NUM_PARTITIONS; curpar++)<br>
@@ -535,6 +581,24 @@<br>
         }<br>
     }<br>
<br>
+    if (opt.pixeladd_ss)<br>
+    {<br>
+        if (!check_pixeladd_ss(ref.pixeladd_ss, opt.pixeladd_ss))<br>
+        {<br>
+            printf("pixel add clip failed!\n");<br>
+            return false;<br>
+        }<br>
+    }<br>
+<br>
+    if (opt.pixeladd_pp)<br>
+    {<br>
+        if (!check_pixeladd_pp(ref.pixeladd_pp, opt.pixeladd_pp))<br>
+        {<br>
+            printf("pixel add clip failed!\n");<br>
+            return false;<br>
+        }<br>
+    }<br>
+<br>
     return true;<br>
 }<br>
<br>
@@ -649,4 +713,16 @@<br>
         printf("Pixel Sub");<br>
         REPORT_SPEEDUP(opt.pixelsub_sp, ref.pixelsub_sp, 64, 64, (short*)pbuf1, FENC_STRIDE, pbuf2, pbuf1, STRIDE, STRIDE);<br>
     }<br>
+<br>
+    if (opt.pixeladd_ss)<br>
+    {<br>
+        printf("pixel_ss add");<br>
+        REPORT_SPEEDUP(opt.pixeladd_ss, ref.pixeladd_ss, 64, 64, (short*)pbuf1, FENC_STRIDE, (short*)pbuf2, (short*)pbuf1, STRIDE, STRIDE);<br>
+    }<br>
+<br>
+    if (opt.pixeladd_pp)<br>
+    {<br>
+        printf("pixel_pp add");<br>
+        REPORT_SPEEDUP(opt.pixeladd_pp, ref.pixeladd_pp, 64, 64, pbuf1, FENC_STRIDE, pbuf2, pbuf1, STRIDE, STRIDE);<br>
+    }<br>
 }<br>
diff -r f813f110d69a -r d93bf22889f8 source/test/pixelharness.h<br>
--- a/source/test/pixelharness.h        Thu Jul 18 02:10:37 2013 -0500<br>
+++ b/source/test/pixelharness.h        Thu Jul 18 12:55:02 2013 +0530<br>
@@ -48,6 +48,8 @@<br>
     bool check_calcrecon(x265::calcrecon_t ref, x265::calcrecon_t opt);<br>
     bool check_weightpUni(x265::weightpUni_t ref, x265::weightpUni_t opt);<br>
     bool check_pixelsub_sp(x265::pixelsub_sp_t ref, x265::pixelsub_sp_t opt);<br>
+    bool check_pixeladd_ss(x265::pixeladd_ss_t ref, x265::pixeladd_ss_t opt);<br>
+    bool check_pixeladd_pp(x265::pixeladd_pp_t ref, x265::pixeladd_pp_t opt);<br>
<br>
 public:<br>
<br>
<br>_______________________________________________<br>
x265-devel mailing list<br>
<a href="mailto:x265-devel@videolan.org">x265-devel@videolan.org</a><br>
<a href="http://mailman.videolan.org/listinfo/x265-devel" target="_blank">http://mailman.videolan.org/listinfo/x265-devel</a><br>
<br></blockquote></div><br><br clear="all"><div><br></div>-- <br>Steve Borho
</div></div>