This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] Power/GCC: Subword atomic operation endianness check bug fix
- From: "Maciej W. Rozycki" <macro at codesourcery dot com>
- To: <gcc-patches at gcc dot gnu dot org>
- Cc: Anton Blanchard <anton at samba dot org>
- Date: Wed, 2 Jul 2014 11:05:23 +0100
- Subject: [PATCH] Power/GCC: Subword atomic operation endianness check bug fix
- Authentication-results: sourceware.org; auth=none
Hi,
This change:
------------------------------------------------------------------------
r199935 | amodra | 2013-06-11 07:17:50 +0100 (Tue, 11 Jun 2013) | 4 lines
* config/rs6000/rs6000.c (rs6000_adjust_atomic_subword): Calculate
correct shift value in little-endian mode.
------------------------------------------------------------------------
fixed subword atomic operations on little-endian Power targets, but while
the effect of the change happens to be correct, the way it is achieved is
not.
This code operates on SImode values, however refers to WORDS_BIG_ENDIAN
to check the endianness. This is not right, quoting our internal manual:
"[...] The memory order of bytes is defined by two
target macros, `WORDS_BIG_ENDIAN' and `BYTES_BIG_ENDIAN':
* `WORDS_BIG_ENDIAN', if set to 1, says that byte number
zero is part of the most significant word; otherwise, it
is part of the least significant word.
* `BYTES_BIG_ENDIAN', if set to 1, says that byte number
zero is the most significant byte within a word;
otherwise, it is the least significant byte within a
word."
so WORDS_BIG_ENDIAN should only ever be referred when operating on
multi-word values, not single-word ones such as SImode ones are. As it
happens both macros have the same value for the Power target, so the
result of the check works out the same, but nevertheless it's not correct
and can be misleading to the casual reader.
The fix below has been regression tested with the powerpc-eabi target and
the following multilibs:
-mcpu=603e
-mcpu=603e -msoft-float
-mcpu=8540 -mfloat-gprs=single -mspe=yes -mabi=spe
-mcpu=8540 -mfloat-gprs=single -mspe=yes -mabi=spe -msoft-float
-mcpu=8548 -mfloat-gprs=double -mspe=yes -mabi=spe
-mcpu=8548 -mfloat-gprs=double -mspe=yes -mabi=spe -mlittle
-mcpu=8548 -mfloat-gprs=double -mspe=yes -mabi=spe -msoft-float
-mcpu=7400 -maltivec -mabi=altivec
as well as the powerpc-linux-gnu target and the following multilibs:
-mcpu=603e
-mcpu=603e -msoft-float
-mcpu=8540 -mfloat-gprs=single -mspe=yes -mabi=spe
-mcpu=8548 -mfloat-gprs=double -mspe=yes -mabi=spe
-mcpu=7400 -maltivec -mabi=altivec
-mcpu=e5500 -m64
OK to apply?
2014-07-02 Maciej W. Rozycki <macro@codesourcery.com>
gcc/
* config/rs6000/rs6000.c (rs6000_adjust_atomic_subword): Use
BYTES_BIG_ENDIAN rather than WORDS_BIG_ENDIAN to check for byte
endianness.
Maciej
gcc-ppc-atomic-le.diff
Index: gcc-fsf-trunk-quilt/gcc/config/rs6000/rs6000.c
===================================================================
--- gcc-fsf-trunk-quilt.orig/gcc/config/rs6000/rs6000.c 2014-06-10 21:46:36.628975329 +0100
+++ gcc-fsf-trunk-quilt/gcc/config/rs6000/rs6000.c 2014-06-11 16:35:08.917560846 +0100
@@ -19891,7 +19891,7 @@ rs6000_adjust_atomic_subword (rtx orig_m
shift = gen_reg_rtx (SImode);
addr = gen_lowpart (SImode, addr);
emit_insn (gen_rlwinm (shift, addr, GEN_INT (3), GEN_INT (shift_mask)));
- if (WORDS_BIG_ENDIAN)
+ if (BYTES_BIG_ENDIAN)
shift = expand_simple_binop (SImode, XOR, shift, GEN_INT (shift_mask),
shift, 1, OPTAB_LIB_WIDEN);
*pshift = shift;