xf86-video-ati: Fix build for xserver < 1.13

From d3bf43ba5ca62dc2409b58cb4f79efe387bf891f Mon Sep 17 00:00:00 2001

Subject: [PATCH] fix build for xserver < 1.13

same procedure every few patches.....

This commit modifies some pointer definitions in functions copying memory corresponding to those in memcpy.
That should enable a compiler to produce better code (though i haven't checked whether that's the case).

src/radeon_accel.c +3 -3

@@ -128,13 +128,13 @@ int radeon_cs_space_remaining(ScrnInfoPtr pScrn)
     return (info->cs->ndw - info->cs->cdw);
 }
 
-void RADEONCopySwap(uint8_t *dst, uint8_t *src, unsigned int size, int
 swap)
+void RADEONCopySwap(uint8_t * __restrict dst, const uint8_t *
 __restrict src, unsigned int size, int swap)
 {
     switch(swap) {
     case RADEON_HOST_DATA_SWAP_32BIT:
         {
-           unsigned int *d = (unsigned int *)dst;
-           unsigned int *s = (unsigned int *)src;
+           unsigned int * __restrict d = (unsigned int *)dst;
+           unsigned int * __restrict s = (unsigned int *)src;
            unsigned int nwords = size >> 2;

            for (; nwords > 0; --nwords, ++d, ++s)

From d39f7a0d42fcc423030cd73bff8afe6d39d13e80 Mon Sep 17 00:00:00 2001

This commit replaces the loop for calculating log base 2 for non-x86-platforms in radeon.h with a clz (count leading zeroes)-based version to simplify the code and, well, eliminate the loop.
Note: There’s no check for val=0 case, since x86-bsr is undefined for that case too, that should be okay.

---
 src/radeon.h |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

src/radeon.h +2 -1

index cbc7866..b1a1ce0 100644
@@ -933,17 +933,16 @@ enum {
 static __inline__ int
 RADEONLog2(int val)
 {
-    int bits;
 #if (defined __i386__ || defined __x86_64__) && (defined __GNUC__)
+    int bits;
+
     __asm volatile("bsrl    %1, %0"
         : "=r" (bits)

Quoted 28 lines

This commit replaces the loop for calculating log base 2 for non-x86-platforms in radeon.h with a clz (count leading zeroes)-based version to simplify the code and, well, eliminate the loop.
Note: There’s no check for val=0 case, since x86-bsr is undefined for that case too, that should be okay.

---
  src/radeon.h |    7 +++----
  1 file changed, 3 insertions(+), 4 deletions(-)

src/radeon.h +2 -2

index cbc7866..b1a1ce0 100644
@@ -933,17 +933,16 @@ enum {
  static __inline__ int
  RADEONLog2(int val)
  {
-    int bits;
  #if (defined __i386__ || defined __x86_64__) && (defined __GNUC__)
+    int bits;
+
      __asm volatile("bsrl    %1, %0"
          : "=r" (bits)
          : "c" (val)
      );
      return bits;
  #else
-    for (bits = 0; val != 0; val >>= 1, ++bits)

src/radeon.h enum { · line 947

Quoted 5 lines

src/radeon.h +1 -2

-        ;
-    return bits - 1;
+    return (31 - __builtin_clz(val));
  #endif
  }

Any reason for not using __builtin_clz on x86 as well? AFAICT both gcc and clang seem to generate more or less the same code with that as with the inline assembly.

Am 29.11.2016 um 08:32 schrieb Michel Dänzer:

Quoted 29 lines · Jochen Rollwagen

Quoted 28 lines

This commit replaces the loop for calculating log base 2 for non-x86-platforms in radeon.h with a clz (count leading zeroes)-based version to simplify the code and, well, eliminate the loop.
Note: There’s no check for val=0 case, since x86-bsr is undefined for that case too, that should be okay.

---
 src/radeon.h |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

src/radeon.h +2 -2

index cbc7866..b1a1ce0 100644
@@ -933,17 +933,16 @@ enum {
 static __inline__ int
 RADEONLog2(int val)
 {
-    int bits;
 #if (defined __i386__ || defined __x86_64__) && (defined __GNUC__)
+    int bits;
+
     __asm volatile("bsrl    %1, %0"
         : "=r" (bits)
         : "c" (val)
     );
     return bits;
 #else
-    for (bits = 0; val != 0; val >>= 1, ++bits)

Quoted 10 lines

src/radeon.h enum { · line 947

Quoted 5 lines

src/radeon.h +1 -2

-        ;
-    return bits - 1;
+    return (31 - __builtin_clz(val));
 #endif
 }

Any reason for not using __builtin_clz on x86 as well? AFAICT both gcc and clang seem to generate more or less the same code with that as with the inline assembly.

I guess not. According to
http://stackoverflow.com/questions/9353973/implementation-of-builtin-clz
"bsr and clz are related but different.

On x86 for clz gcc (-O2) generates:

bsrl %edi, %eax
xorl $31, %eax
ret "

Quoted 44 lines

Am 29.11.2016 um 08:32 schrieb Michel Dänzer:

Quoted 29 lines · Jochen Rollwagen

Quoted 28 lines

This commit replaces the loop for calculating log base 2 for non-x86-platforms in radeon.h with a clz (count leading zeroes)-based version to simplify the code and, well, eliminate the loop.
Note: There’s no check for val=0 case, since x86-bsr is undefined for that case too, that should be okay.

---
  src/radeon.h |    7 +++----
  1 file changed, 3 insertions(+), 4 deletions(-)

src/radeon.h +2 -2

index cbc7866..b1a1ce0 100644
@@ -933,17 +933,16 @@ enum {
  static __inline__ int
  RADEONLog2(int val)
  {
-    int bits;
  #if (defined __i386__ || defined __x86_64__) && (defined __GNUC__)
+    int bits;
+
      __asm volatile("bsrl    %1, %0"
          : "=r" (bits)
          : "c" (val)
      );
      return bits;
  #else
-    for (bits = 0; val != 0; val >>= 1, ++bits)

Quoted 10 lines

src/radeon.h enum { · line 947

Quoted 5 lines

src/radeon.h +1 -2

-        ;
-    return bits - 1;
+    return (31 - __builtin_clz(val));
  #endif
  }

Any reason for not using __builtin_clz on x86 as well? AFAICT both gcc and clang seem to generate more or less the same code with that as with the inline assembly.

I guess not. According to
http://stackoverflow.com/questions/9353973/implementation-of-builtin-clz
"bsr and clz are related but different.

Quoted 5 lines

On x86 for clz gcc (-O2) generates:

bsrl %edi, %eax
xorl $31, %eax
ret "

That's not what I'm seeing. Have you compared the code generated by your compiler in both cases?