Jeff Law wrote:
On 07/08/2015 03:43 AM, Richard Biener wrote:
On Wed, Jul 8, 2015 at 12:07 AM, Jeff Law <l...@redhat.com> wrote:
On 07/07/2015 06:37 AM, Alan Lawrence wrote:
As per https://gcc.gnu.org/ml/gcc-patches/2015-04/msg01346.html. Fixes
FAIL of advsimd-intrinsics vcreate.c on aarch64_be-none-elf from
previous patch.

15_native_interpret_real.patch


commit e2e7ca148960a82fc88128820f17e7cbd14173cb
Author: Alan Lawrence<alan.lawre...@arm.com>
Date:   Thu Apr 9 10:54:40 2015 +0100

      Fix native_interpret_real for HFmode floats on Bigendian with
UNITS_PER_WORD>=4

      (with missing space)
OK with ChangeLog in proper form.
Err - but now offset can become negative?  Shouldn't it rather error out
before as it requires at least 4 bytes for big-endian?
I managed to convince myself the value wouldn't be negative when reviewing.

That said - the whole thing looks it doesn't expect GET_MODE_SIZE < 4
and your "fix" is just very obfuscated (if it really is a fix).
While I couldn't convince myself the function as a whole was prepared for smaller objects, I don't think Alan's patch made things worse. One could argue the whole bloody thing ought to be rewritten though.

I'd also managed to convince myself the other instances of "3" weren't problematical.

jeff


In response to Richard's comments, may I propose the attached patch instead?

I used the term "long" because of the earlier comment:
  /* There are always 32 bits in each long, no matter the size of
     the hosts long.  We handle floating point representations with
     up to 192 bits.  */
with which I really don't think I want to mess at this point ;). However, I'd be happy to change my use of "long" to "32 bits", "4 bytes", or "group of 4" instead if one of those was preferable!

Bootstrapped + check-gcc on x86_64 (no change); cross-tested on aarch64_be-none-elf (where it fixes the advsimd-intrinsics float16 test)

gcc/ChangeLog (as before):

        * fold-const.c (native_interpret_real): Fix HFmode for bigendian where
        UNITS_PER_WORD >= 4.
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index e61d9463462746f45c96a0e7a154fb45ed026f43..518780e6cd2724002f0c7a805aa7742b6374600c 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -7592,7 +7592,6 @@ native_interpret_real (tree type, const unsigned char *ptr, int len)
 {
   machine_mode mode = TYPE_MODE (type);
   int total_bytes = GET_MODE_SIZE (mode);
-  int byte, offset, word, words, bitpos;
   unsigned char value;
   /* There are always 32 bits in each long, no matter the size of
      the hosts long.  We handle floating point representations with
@@ -7603,16 +7602,18 @@ native_interpret_real (tree type, const unsigned char *ptr, int len)
   total_bytes = GET_MODE_SIZE (TYPE_MODE (type));
   if (total_bytes > len || total_bytes > 24)
     return NULL_TREE;
-  words = (32 / BITS_PER_UNIT) / UNITS_PER_WORD;
+  int words = (32 / BITS_PER_UNIT) / UNITS_PER_WORD;
 
   memset (tmp, 0, sizeof (tmp));
-  for (bitpos = 0; bitpos < total_bytes * BITS_PER_UNIT;
+  for (int bitpos = 0; bitpos < total_bytes * BITS_PER_UNIT;
        bitpos += BITS_PER_UNIT)
     {
-      byte = (bitpos / BITS_PER_UNIT) & 3;
+      /* Both OFFSET and BYTE index within a long;
+	 bitpos indexes the whole float.  */
+      int offset, byte = (bitpos / BITS_PER_UNIT) & 3;
       if (UNITS_PER_WORD < 4)
 	{
-	  word = byte / UNITS_PER_WORD;
+	  int word = byte / UNITS_PER_WORD;
 	  if (WORDS_BIG_ENDIAN)
 	    word = (words - 1) - word;
 	  offset = word * UNITS_PER_WORD;
@@ -7622,7 +7623,16 @@ native_interpret_real (tree type, const unsigned char *ptr, int len)
 	    offset += byte % UNITS_PER_WORD;
 	}
       else
-	offset = BYTES_BIG_ENDIAN ? 3 - byte : byte;
+	{
+	  offset = byte;
+	  if (BYTES_BIG_ENDIAN)
+	    {
+	      /* Reverse bytes within each long, or within the entire float
+		 if it's smaller than a long (for HFmode).  */
+	      offset = MIN (3, total_bytes - 1) - offset;
+	      gcc_assert (offset >= 0);
+	    }
+	}
       value = ptr[offset + ((bitpos / BITS_PER_UNIT) & ~3)];
 
       tmp[bitpos / 32] |= (unsigned long)value << (bitpos & 31);

Reply via email to