Re: [FFmpeg-devel] [PATCH 2/3] checkasm/blockdsp: use smallest allowed aligned buffers for fill_block_tab tests

[James Almer]

On 5/7/2024 12:14 PM, Andreas Rheinhardt wrote:

James Almer:

The requirement is either 8 or 16 bytes alignment, not 32.
This should help finding bugs in asm implementations.

Signed-off-by: James Almer <jamr...@gmail.com>
---
  tests/checkasm/blockdsp.c | 23 +++++++++--------------
  1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/tests/checkasm/blockdsp.c b/tests/checkasm/blockdsp.c
index ab87fc8fa4..f67a38d302 100644
--- a/tests/checkasm/blockdsp.c
+++ b/tests/checkasm/blockdsp.c
@@ -29,11 +29,6 @@
  #include "libavutil/intreadwrite.h"
  #include "libavutil/mem_internal.h"

-typedef struct {

-    const char *name;
-    int size;
-} test;
-
  #define randomize_buffers(size)             \
      do {                                    \
          int i;                              \
@@ -58,18 +53,18 @@ do {                                                        
        \
  } while (0)

static void check_fill(BlockDSPContext *h){

-    const test tests[] = {
-        {"fill_block_tab[0]", 16},
-        {"fill_block_tab[1]", 8},
-    };
-    LOCAL_ALIGNED_32(uint8_t, buf0, [16 * 16]);
-    LOCAL_ALIGNED_32(uint8_t, buf1, [16 * 16]);
+    LOCAL_ALIGNED_16(uint8_t, buf0_16, [16 * 16]);
+    LOCAL_ALIGNED_16(uint8_t, buf1_16, [16 * 16]);
+    LOCAL_ALIGNED_8(uint8_t, buf0_8, [8 * 8]);
+    LOCAL_ALIGNED_8(uint8_t, buf1_8, [8 * 8]);

- for (size_t t = 0; t < FF_ARRAY_ELEMS(tests); ++t) {

-        int n = tests[t].size;
+    for (int t = 0; t < 2; ++t) {
+        uint8_t *buf0 = t ? buf0_8 : buf0_16;
+        uint8_t *buf1 = t ? buf1_8 : buf1_16;
+        int n = 16 - 8 * t;
          declare_func(void, uint8_t *block, uint8_t value,
                       ptrdiff_t line_size, int h);
-        if (check_func(h->fill_block_tab[t], "blockdsp.%s", tests[t].name)) {
+        if (check_func(h->fill_block_tab[t], "blockdsp.fill_block_tab[%d]", 
t)) {
              uint8_t value = rnd();
              memset(buf0, 0, sizeof(*buf0) * n * n);
              memset(buf1, 0, sizeof(*buf0) * n * n);

1. I wouldn't be surprised if the *_8 buffers were still 16 byte
aligned. You should probably force 8 byte alignment by using a 16
byte-aligned buffer with an offset of eight.

Amended the following locally:

diff --git a/tests/checkasm/blockdsp.c b/tests/checkasm/blockdsp.c
index 8c1f8281d2..5f4d46b8fa 100644
--- a/tests/checkasm/blockdsp.c
+++ b/tests/checkasm/blockdsp.c
@@ -55,12 +55,10 @@ do {                                                        
        \
 static void check_fill(BlockDSPContext *h){
     LOCAL_ALIGNED_16(uint8_t, buf0_16, [16 * 16]);
     LOCAL_ALIGNED_16(uint8_t, buf1_16, [16 * 16]);
-    LOCAL_ALIGNED_8(uint8_t, buf0_8, [8 * 8]);
-    LOCAL_ALIGNED_8(uint8_t, buf1_8, [8 * 8]);

     for (int t = 0; t < 2; ++t) {
-        uint8_t *buf0 = t ? buf0_8 : buf0_16;
-        uint8_t *buf1 = t ? buf1_8 : buf1_16;
+        uint8_t *buf0 = buf0_16 + t * /* force 8 byte alignment */ 8;
+        uint8_t *buf1 = buf1_16 + t * /* force 8 byte alignment */ 8;
         int n = 16 - 8 * t;
         declare_func(void, uint8_t *block, uint8_t value,
                      ptrdiff_t line_size, int h);

2. Can you also extend this test to actually test the case of stride !=
width? (And negative strides.)

Maybe later.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".