[PATCH] Power/GCC: Subword atomic operation endianness check bug fix

Maciej W. Rozycki macro@codesourcery.com
Wed Jul 2 10:05:00 GMT 2014


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;



More information about the Gcc-patches mailing list