On 05.10.2017 20:59, Jochen Rollwagen wrote:
Am 04.10.2017 um 05:59 schrieb Matt Turner:
On Tue, Oct 3, 2017 at 11:01 AM, Jochen Rollwagen
<joro-2...@t-online.de> wrote:
From 4cebe50a9bade6717923e104c954f3fad75f71bb Mon Sep 17 00:00:00 2001
From: Jochen Rollwagen <joro-2...@t-online.de>
Date: Tue, 3 Oct 2017 19:54:10 +0200
Subject: [PATCH] Replace byte-swapping code with builtins in pack.c
This patch replaces some code for byte-swapping in pack.c with the
builtin
functions allowing the compiler to do its optimization magic
---
src/mesa/main/pack.c | 22 ++--------------------
1 file changed, 2 insertions(+), 20 deletions(-)
diff --git a/src/mesa/main/pack.c b/src/mesa/main/pack.c
index 94a6d28..9bfde39 100644
--- a/src/mesa/main/pack.c
+++ b/src/mesa/main/pack.c
@@ -230,26 +230,8 @@ _mesa_pack_bitmap( GLint width, GLint height, const
GLubyte
*source,
}
}
-
-#define SWAP2BYTE(VALUE) \
- { \
- GLubyte *bytes = (GLubyte *) &(VALUE); \
- GLubyte tmp = bytes[0]; \
- bytes[0] = bytes[1]; \
- bytes[1] = tmp; \
- }
-
-#define SWAP4BYTE(VALUE) \
- { \
- GLubyte *bytes = (GLubyte *) &(VALUE); \
- GLubyte tmp = bytes[0]; \
- bytes[0] = bytes[3]; \
- bytes[3] = tmp; \
- tmp = bytes[1]; \
- bytes[1] = bytes[2]; \
- bytes[2] = tmp; \
- }
-
+#define SWAP2BYTE(VALUE) __builtin_bswap16(VALUE)
+#define SWAP4BYTE(VALUE) __builtin_bswap32(VALUE)
In my experience it's much simpler to just write these as
return ((x & 0xff) << 8) | ((x >> 8) & 0xff);
and
return ((x & 0xff) << 24) |
((x & 0xff00) << 8) |
((x & 0xff0000) >> 8) |
((x >> 24) & 0xff);
and not have to deal with compiler intrinsics. Compilers will
recognize these patterns and use the appropriate instructions (rol for
2-bytes and bswap for 4-bytes).
You should be able to count the numbers of those instructions before
and after such a patch to confirm it's working as expected.
Fair enough. I've attached a new patch that optimizes the code on linux
systems only where there is byteswap.h containing (hopefully optimal)
functions. The other systems may be dealt with by a followup patch if
desired.
This doesn't really address Matt's point that compilers are good at
pattern matching byte swaps already.
Unless you can actually show with comparisons of the assembly and/or
benchmarks that this results in better code, your patch makes the code
base slightly worse because you've now added two different code paths
where there was previously only one.
Cheers,
Nicolai
From 327012410f75f77010b658ce9643a229c45bc308 Mon Sep 17 00:00:00 2001
From: Jochen Rollwagen <joro-2...@t-online.de>
Date: Thu, 5 Oct 2017 20:48:46 +0200
Subject: [PATCH] Simplify byte swapping code in pack.c on Linux systems
This patch replaces the code for byte swapping in pack.c with the
function from
byteswap.h on Linux systems
---
src/mesa/main/pack.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/mesa/main/pack.c b/src/mesa/main/pack.c
index 94a6d28..d8ab267 100644
--- a/src/mesa/main/pack.c
+++ b/src/mesa/main/pack.c
@@ -230,6 +230,12 @@ _mesa_pack_bitmap( GLint width, GLint height, const
GLubyte
*source,
}
}
+#ifdef __linux__
+#include <byteswap.h>
+
+#define SWAP2BYTE(VALUE) bswap_16(VALUE)
+#define SWAP4BYTE(VALUE) bswap_32(VALUE)
+#else
#define SWAP2BYTE(VALUE) \
{ \
@@ -249,7 +255,7 @@ _mesa_pack_bitmap( GLint width, GLint height, const
GLubyte
*source,
bytes[1] = bytes[2]; \
bytes[2] = tmp; \
}
-
+#endif
static void
extract_uint_indexes(GLuint n, GLuint indexes[],
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
