Hi,
bswap optimization pass generate wrong code on big endian targets when the
result of a bit operation it analyzed is a partial load of the range of memory
accessed by the original expression (when one or more bytes at lowest address
were lost in the computation). This is due to the way cmpxchg and cmpnop are
adjusted in find_bswap_or_nop before being compared to the result of the
symbolic expression. Part of the adjustment is endian independent: it's to
ignore the bytes that were not accessed by the original gimple expression.
However, when the result has less byte than that original expression, some
more byte need to be ignored and this is endian dependent.
The current code only support loss of bytes at the highest addresses because
there is no code to adjust the address of the load. However, for little and
big endian targets the bytes at highest address translate into different byte
significance in the result. This patch first separate cmpxchg and cmpnop
adjustement into 2 steps and then deal with endianness correctly for the
second step.
ChangeLog entries are as follow:
*** gcc/ChangeLog ***
2015-12-16 Thomas Preud'homme <thomas.preudho...@arm.com>
PR tree-optimization/67781
* tree-ssa-math-opts.c (find_bswap_or_nop): Zero out bytes in cmpxchg
and cmpnop in two steps: first the ones not accessed in original
gimple expression in a endian independent way and then the ones not
accessed in the final result in an endian-specific way.
*** gcc/testsuite/ChangeLog ***
2015-12-16 Thomas Preud'homme <thomas.preudho...@arm.com>
PR tree-optimization/67781
* gcc.c-torture/execute/pr67781.c: New file.
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr67781.c
b/gcc/testsuite/gcc.c-torture/execute/pr67781.c
new file mode 100644
index 0000000..bf50aa2
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr67781.c
@@ -0,0 +1,34 @@
+#ifdef __UINT32_TYPE__
+typedef __UINT32_TYPE__ uint32_t;
+#else
+typedef unsigned uint32_t;
+#endif
+
+#ifdef __UINT8_TYPE__
+typedef __UINT8_TYPE__ uint8_t;
+#else
+typedef unsigned char uint8_t;
+#endif
+
+struct
+{
+ uint32_t a;
+ uint8_t b;
+} s = { 0x123456, 0x78 };
+
+int pr67781()
+{
+ uint32_t c = (s.a << 8) | s.b;
+ return c;
+}
+
+int
+main ()
+{
+ if (sizeof (uint32_t) * __CHAR_BIT__ != 32)
+ return 0;
+
+ if (pr67781 () != 0x12345678)
+ __builtin_abort ();
+ return 0;
+}
diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
index b00f046..e5a185f 100644
--- a/gcc/tree-ssa-math-opts.c
+++ b/gcc/tree-ssa-math-opts.c
@@ -2441,6 +2441,8 @@ find_bswap_or_nop_1 (gimple *stmt, struct symbolic_number
*n, int limit)
static gimple *
find_bswap_or_nop (gimple *stmt, struct symbolic_number *n, bool *bswap)
{
+ unsigned rsize;
+ uint64_t tmpn, mask;
/* The number which the find_bswap_or_nop_1 result should match in order
to have a full byte swap. The number is shifted to the right
according to the size of the symbolic number before using it. */
@@ -2464,24 +2466,38 @@ find_bswap_or_nop (gimple *stmt, struct symbolic_number
*n, bool *bswap)
/* Find real size of result (highest non-zero byte). */
if (n->base_addr)
- {
- int rsize;
- uint64_t tmpn;
-
- for (tmpn = n->n, rsize = 0; tmpn; tmpn >>= BITS_PER_MARKER, rsize++);
- n->range = rsize;
- }
+ for (tmpn = n->n, rsize = 0; tmpn; tmpn >>= BITS_PER_MARKER, rsize++);
+ else
+ rsize = n->range;
- /* Zero out the extra bits of N and CMP*. */
+ /* Zero out the bits corresponding to untouched bytes in original gimple
+ expression. */
if (n->range < (int) sizeof (int64_t))
{
- uint64_t mask;
-
mask = ((uint64_t) 1 << (n->range * BITS_PER_MARKER)) - 1;
cmpxchg >>= (64 / BITS_PER_MARKER - n->range) * BITS_PER_MARKER;
cmpnop &= mask;
}
+ /* Zero out the bits corresponding to unused bytes in the result of the
+ gimple expression. */
+ if (rsize < n->range)
+ {
+ if (BYTES_BIG_ENDIAN)
+ {
+ mask = ((uint64_t) 1 << (rsize * BITS_PER_MARKER)) - 1;
+ cmpxchg &= mask;
+ cmpnop >>= (n->range - rsize) * BITS_PER_MARKER;
+ }
+ else
+ {
+ mask = ((uint64_t) 1 << (rsize * BITS_PER_MARKER)) - 1;
+ cmpxchg >>= (n->range - rsize) * BITS_PER_MARKER;
+ cmpnop &= mask;
+ }
+ n->range = rsize;
+ }
+
/* A complete byte swap should make the symbolic number to start with
the largest digit in the highest order byte. Unchanged symbolic
number indicates a read with same endianness as target architecture. */
Regression testsuite was run on a bootstrapped native x86_64-linux-gnu GCC and
on an arm-none-eabi GCC cross-compiler without any regression. I'm waiting for
a slot on gcc110 to do a big endian bootstrap but at least the testcase works
on mips-linux. I'll send an update once bootstrap is complete.
Is this ok for trunk and 5 branch in a week time if no regression is reported?
Best regards,
Thomas
