Re: Add -fsection-anchors (4.2 project)

[Richard Sandiford <richard at codesourcery dot com>]

Mark Mitchell <mark@codesourcery.com> writes:
>> +void
>> +rtl_check_failed_block_symbol (const char *file, int line, const char *func)
>
> Nit: this new function should have a comment.

Fixed.

I also noticed a few problems with the offset calculation code in
get_section_anchor:

  - It didn't detect the case where the offset range was the same
    as the range of a HOST_WIDE_INT.  We'd get division by zero
    in that case.

  - Related, "targetm.max_anchor_offset - targetm.min_anchor_offset + 1"
    could lead to signed overflow.

  - "offset - targetm.min_anchor_offset" could lead to signed overflow.

  - We didn't cope very well with offsets that were outside the range
    of the regularly-spaced, 0-based anchors.

The patch below should fix these problems and was approved off-list by Mark.
As well as the usual testing, I copied the new code to a self-contained
program and made sure that it behaved as expected for some off-beat
values of GET_MODE_BITSIZE (ptr_mode) and {min,max}_anchor_offset.

David Edelsohn thought that using a 16-bit offset was too restrictive.
I suppose it probably is for PowerPC, where addis provides a cheap
way of adding a 64k offset to an anchor (both in terms of size and
speed).  We settled on a 32-bit offset instead; this part was
approved off-list by David.  As a result:

    static int x[0x10000];
    static int y,z;
    int foo (voiD) { return x[0] + x[0xffff] + y + z; }

produces the following code when compiled with -O2 -fsection-anchors:

.L.foo:
        ld 11,.LC0@toc(2)
        addis 9,11,0x4
        lwz 10,0(11)
        lwz 3,-4(9)
        lwz 0,0(9)
        lwz 11,4(9)
        add 3,3,10
        add 3,3,0
        add 3,3,11
        extsw 3,3
        blr

The SPEC results were very similar to before (i.e. all differences
appeared to be in noise range).

I bootstrapped & regression tested the patch on powerpc64-linux-gnu
with the additional changes below (which are relative to the original
patch, and covered by the same ChangeLog).  Applied to trunk.

Richard


diff -upr gcc.old/config/rs6000/rs6000.c gcc/config/rs6000/rs6000.c
--- gcc/config/rs6000/rs6000.c	2006-02-18 01:42:42.000000000 -0800
+++ gcc/config/rs6000/rs6000.c	2006-02-16 03:56:44.000000000 -0800
@@ -1022,10 +1022,17 @@ static const char alt_reg_names[][8] =
 #define TARGET_ASM_OUTPUT_DWARF_DTPREL rs6000_output_dwarf_dtprel
 #endif
 
+/* Use a 32-bit anchor range.  This leads to sequences like:
+
+	addis	tmp,anchor,high
+	add	dest,tmp,low
+
+   where tmp itself acts as an anchor, and can be shared between
+   accesses to the same 64k page.  */
 #undef TARGET_MIN_ANCHOR_OFFSET
-#define TARGET_MIN_ANCHOR_OFFSET -32768
+#define TARGET_MIN_ANCHOR_OFFSET -0x7fffffff - 1
 #undef TARGET_MAX_ANCHOR_OFFSET
-#define TARGET_MAX_ANCHOR_OFFSET 32767
+#define TARGET_MAX_ANCHOR_OFFSET 0x7fffffff
 #undef TARGET_USE_BLOCKS_FOR_CONSTANT_P
 #define TARGET_USE_BLOCKS_FOR_CONSTANT_P rs6000_use_blocks_for_constant_p
 
diff -upr gcc.old/rtl.c gcc/rtl.c
--- gcc/rtl.c	2006-02-18 01:42:52.000000000 -0800
+++ gcc/rtl.c	2006-02-16 03:56:44.000000000 -0800
@@ -536,6 +536,9 @@ rtl_check_failed_code_mode (rtx r, enum 
 		  func, trim_filename (file), line);
 }
 
+/* Report that line LINE of FILE tried to access the block symbol fields
+   of a non-block symbol.  FUNC is the function that contains the line.  */
+
 void
 rtl_check_failed_block_symbol (const char *file, int line, const char *func)
 {
diff -upr gcc.old/varasm.c gcc/varasm.c
--- gcc/varasm.c	2006-02-18 01:42:42.000000000 -0800
+++ gcc/varasm.c	2006-02-18 00:17:17.000000000 -0800
@@ -6004,15 +6004,48 @@ get_section_anchor (struct object_block 
 {
   char label[100];
   unsigned int begin, middle, end;
+  unsigned HOST_WIDE_INT min_offset, max_offset, range, bias, delta;
   rtx anchor;
 
   /* Work out the anchor's offset.  Use an offset of 0 for the first
      anchor so that we don't pessimize the case where we take the address
      of a variable at the beginning of the block.  This is particularly
-     useful when a block has only one variable assigned to it.  */
-  offset = ((offset - targetm.min_anchor_offset)
-	    / (targetm.max_anchor_offset - targetm.min_anchor_offset + 1)
-	    * (targetm.max_anchor_offset - targetm.min_anchor_offset + 1));
+     useful when a block has only one variable assigned to it.
+
+     We try to place anchors RANGE bytes apart, so there can then be
+     anchors at +/-RANGE, +/-2 * RANGE, and so on, up to the limits of
+     a ptr_mode offset.  With some target settings, the lowest such
+     anchor might be out of range for the lowest ptr_mode offset;
+     likewise the highest anchor for the highest offset.  Use anchors
+     at the extreme ends of the ptr_mode range in such cases.
+
+     All arithmetic uses unsigned integers in order to avoid
+     signed overflow.  */
+  max_offset = (unsigned HOST_WIDE_INT) targetm.max_anchor_offset;
+  min_offset = (unsigned HOST_WIDE_INT) targetm.min_anchor_offset;
+  range = max_offset - min_offset + 1;
+  if (range == 0)
+    offset = 0;
+  else
+    {
+      bias = 1 << (GET_MODE_BITSIZE (ptr_mode) - 1);
+      if (offset < 0)
+	{
+	  delta = -(unsigned HOST_WIDE_INT) offset + max_offset;
+	  delta -= delta % range;
+	  if (delta > bias)
+	    delta = bias;
+	  offset = (HOST_WIDE_INT) (-delta);
+	}
+      else
+	{
+	  delta = (unsigned HOST_WIDE_INT) offset - min_offset;
+	  delta -= delta % range;
+	  if (delta > bias - 1)
+	    delta = bias - 1;
+	  offset = (HOST_WIDE_INT) delta;
+	}
+    }
 
   /* Do a binary search to see if there's already an anchor we can use.
      Set BEGIN to the new anchor's index if not.  */