Re: wide-int, rtl-2

[Eric Botcazou <ebotcazou at adacore dot com>]

> This looks OK to me.

I obviously didn't look carefully enough, there are a few thinkos in the 
output_constructor_bitfield hunk:

-	  if (shift < HOST_BITS_PER_WIDE_INT
-	      && shift + this_time > HOST_BITS_PER_WIDE_INT)
-	    {
-	      this_time = shift + this_time - HOST_BITS_PER_WIDE_INT;
-	      shift = HOST_BITS_PER_WIDE_INT;
-	    }
+	  if ((shift / HOST_BITS_PER_WIDE_INT)
+ 	      != ((shift + this_time) / HOST_BITS_PER_WIDE_INT))
+	    this_time = (shift + this_time) & (HOST_BITS_PER_WIDE_INT - 1);

The tests aren't equivalent since the original one is false e.g. if shift == 1 
and shift + this_time == HOST_BITS_PER_WIDE_INT, but not the new one.  The new 
computation for this_time is not fully correct since it could yield zero but 
may not.  And the "shift = HOST_BITS_PER_WIDE_INT;" line shouldn't have been 
dropped but adjusted.


-	  if (shift < HOST_BITS_PER_WIDE_INT
-	      && shift + this_time > HOST_BITS_PER_WIDE_INT)
+	  if ((shift / HOST_BITS_PER_WIDE_INT)
+	      != ((shift + this_time) / HOST_BITS_PER_WIDE_INT))
 	    this_time = (HOST_BITS_PER_WIDE_INT - shift);

Likewise for the test.  And the assignment should have been adjusted since 
this_time may not be zero or negative.

This fixes:

WARNING: program timed out.
FAIL: gnat.dg/nested_agg_bitfield_constructor.adb (test for excess errors)
WARNING: program timed out.
FAIL: gnat.dg/outer_agg_bitfield_constructor.adb (test for excess errors)
WARNING: gnat.dg/outer_agg_bitfield_constructor.adb compilation failed to 
produce executable

on big-endian platforms.

Tested on x86_64-suse-linux and SPARC/Solaris, applied on the mainline.


2014-05-23  Eric Botcazou  <ebotcazou@adacore.com>

	* varasm.c (output_constructor_bitfield): Fix thinkos in latest change.


-- 
Eric Botcazou

Index: varasm.c
===================================================================
--- varasm.c	(revision 210676)
+++ varasm.c	(working copy)
@@ -5082,24 +5082,27 @@ output_constructor_bitfield (oc_local_st
       this_time = MIN (end_offset - next_offset, BITS_PER_UNIT - next_bit);
       if (BYTES_BIG_ENDIAN)
 	{
-	  /* On big-endian machine, take the most significant bits
-	     first (of the bits that are significant)
-	     and put them into bytes from the most significant end.  */
+	  /* On big-endian machine, take the most significant bits (of the
+	     bits that are significant) first and put them into bytes from
+	     the most significant end.  */
 	  shift = end_offset - next_offset - this_time;
 
 	  /* Don't try to take a bunch of bits that cross
-	     the word boundary in the INTEGER_CST. We can
-	     only select bits from the LOW or HIGH part
-	     not from both.  */
+	     the word boundary in the INTEGER_CST.  We can
+	     only select bits from one element.  */
 	  if ((shift / HOST_BITS_PER_WIDE_INT)
- 	      != ((shift + this_time) / HOST_BITS_PER_WIDE_INT))
-	    this_time = (shift + this_time) & (HOST_BITS_PER_WIDE_INT - 1);
+	      != ((shift + this_time - 1) / HOST_BITS_PER_WIDE_INT))
+	    {
+	      const int end = shift + this_time - 1;
+	      shift = end & -HOST_BITS_PER_WIDE_INT;
+	      this_time = end - shift + 1;
+	    }
 
 	  /* Now get the bits from the appropriate constant word.  */
 	  value = TREE_INT_CST_ELT (local->val, shift / HOST_BITS_PER_WIDE_INT);
 	  shift = shift & (HOST_BITS_PER_WIDE_INT - 1);
 
-	  /* Get the result. This works only when:
+	  /* Get the result.  This works only when:
 	     1 <= this_time <= HOST_BITS_PER_WIDE_INT.  */
 	  local->byte |= (((value >> shift)
 			   & (((HOST_WIDE_INT) 2 << (this_time - 1)) - 1))
@@ -5107,25 +5110,24 @@ output_constructor_bitfield (oc_local_st
 	}
       else
 	{
-	  /* On little-endian machines,
-	     take first the least significant bits of the value
-	     and pack them starting at the least significant
+	  /* On little-endian machines, take the least significant bits of
+	     the value first and pack them starting at the least significant
 	     bits of the bytes.  */
 	  shift = next_offset - byte_relative_ebitpos;
 
 	  /* Don't try to take a bunch of bits that cross
-	     the word boundary in the INTEGER_CST. We can
-	     only select bits from the LOW or HIGH part
-	     not from both.  */
+	     the word boundary in the INTEGER_CST.  We can
+	     only select bits from one element.  */
 	  if ((shift / HOST_BITS_PER_WIDE_INT)
-	      != ((shift + this_time) / HOST_BITS_PER_WIDE_INT))
-	    this_time = (HOST_BITS_PER_WIDE_INT - shift);
+	      != ((shift + this_time - 1) / HOST_BITS_PER_WIDE_INT))
+	    this_time
+	      = HOST_BITS_PER_WIDE_INT - (shift & (HOST_BITS_PER_WIDE_INT - 1));
 
 	  /* Now get the bits from the appropriate constant word.  */
 	  value = TREE_INT_CST_ELT (local->val, shift / HOST_BITS_PER_WIDE_INT);
 	  shift = shift & (HOST_BITS_PER_WIDE_INT - 1);
 
-	  /* Get the result. This works only when:
+	  /* Get the result.  This works only when:
 	     1 <= this_time <= HOST_BITS_PER_WIDE_INT.  */
 	  local->byte |= (((value >> shift)
 			   & (((HOST_WIDE_INT) 2 << (this_time - 1)) - 1))