This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Fix ivopts address space confusion


This is a followup fix for the patches from PR41857, which contained a bug that is exposed by the ptx backend. The problem is finding the right pointer type for a mem_ref during ivopts.

The original problem was that on spu-elf, __ea pointers are 64 bit while sizetype is 32 bit. During ivopts, pointer types are typically replaced by unsigned integers in computations, and in the original testcase we have an aff_tree containing only one long long element, but no pointer base, and addr_to_parts casts everything to sizetype, losing parts of the address.

To fix that, code was introduced to promote one of the elements of the aff_tree to become the base, cast it to a pointer of an appropriate type, and thus avoid the conversion to sizetype for that element. This element is the base_hint, which is chosen based on the candidate's base_object.

The problem lies in the following code in move_hint_to_base:

  /* Cast value to appropriate pointer type.  We cannot use a pointer
     to TYPE directly, as the back-end will assume registers of pointer
     type are aligned, and just the base itself may not actually be.
     We use void pointer to the type's address space instead.  */
  qual = ENCODE_QUAL_ADDR_SPACE (TYPE_ADDR_SPACE (type));
  type = build_qualified_type (void_type_node, qual);
  parts->base = fold_convert (build_pointer_type (type), val);

This is confused about address spaces. We can get here with either an integer (for the the original testcase in 4.5, it's a long long that holds the pointer value), but I think we can also arrive here with a pointer. Assuming a normal machine, if it's a long long created by ivopts, TYPE_ADDR_SPACE should be just generic, and if it's some other pointer, we should be looking at TYPE_ADDR_SPACE (TREE_TYPE (type)). Thus, the code seems essentially unnecessary, but - up to this point - harmless.

On ptx however, local variables have an address space, and the above code picks that up and incorrectly places it on the pointer destination type.

The following patch reworks this area - instead of trying to find a proper pointer type, it just recognizes the case where an integer is promoted to be the base, and performs all calculations in that type rather than sizetype. That also seems to be an additional bugfix over the original change, which could still lose address bits in the index. I've verified that this produces the same assembly for the original testcase on spu-elf with the gcc-4_5-branch, and it solves the problems I was seeing on ptx.

Bootstrapped and tested on x86_64-linux. Ok?


Bernd
	* tree-ssa-address.c (move_hint_to_base): Remove arg TYPE.  All
	callers changed.  Don't cast the new base to a pointer type.
	(addr_to_parts): If the base has an integer type, use that instead
	of sizetype for calculations.

Index: gcc/tree-ssa-address.c
===================================================================
--- gcc/tree-ssa-address.c	(revision 436508)
+++ gcc/tree-ssa-address.c	(working copy)
@@ -443,12 +443,10 @@ move_fixed_address_to_symbol (struct mem
 /* If ADDR contains an instance of BASE_HINT, move it to PARTS->base.  */
 
 static void
-move_hint_to_base (tree type, struct mem_address *parts, tree base_hint,
-		   aff_tree *addr)
+move_hint_to_base (struct mem_address *parts, tree base_hint, aff_tree *addr)
 {
   unsigned i;
   tree val = NULL_TREE;
-  int qual;
 
   for (i = 0; i < addr->n; i++)
     {
@@ -463,13 +461,7 @@ move_hint_to_base (tree type, struct mem
   if (i == addr->n)
     return;
 
-  /* Cast value to appropriate pointer type.  We cannot use a pointer
-     to TYPE directly, as the back-end will assume registers of pointer
-     type are aligned, and just the base itself may not actually be.
-     We use void pointer to the type's address space instead.  */
-  qual = ENCODE_QUAL_ADDR_SPACE (TYPE_ADDR_SPACE (type));
-  type = build_qualified_type (void_type_node, qual);
-  parts->base = fold_convert (build_pointer_type (type), val);
+  parts->base = val;
   aff_combination_remove_elt (addr, i);
 }
 
@@ -639,7 +631,7 @@ addr_to_parts (tree type, aff_tree *addr
 	       tree base_hint, struct mem_address *parts,
                bool speed)
 {
-  tree part;
+  tree part, int_type;
   unsigned i;
 
   parts->symbol = NULL_TREE;
@@ -671,21 +663,29 @@ addr_to_parts (tree type, aff_tree *addr
      there is no reliable way how to distinguish between pointer and its
      offset, this is just a guess.  */
   if (!parts->symbol && base_hint)
-    move_hint_to_base (type, parts, base_hint, addr);
+    move_hint_to_base (parts, base_hint, addr);
   if (!parts->symbol && !parts->base)
     move_pointer_to_base (parts, addr);
 
+  /* The call to move_hint_to_base may have promoted an integer to the base.
+     Since addresses in different address spaces can have different sizes, do
+     not cast to sizetype in that case, but use the type of the base.  */
+  if (parts->base && TREE_CODE (TREE_TYPE (parts->base)) == INTEGER_TYPE)
+    int_type = TREE_TYPE (parts->base);
+  else
+    int_type = sizetype;
+
   /* Then try to process the remaining elements.  */
   for (i = 0; i < addr->n; i++)
     {
-      part = fold_convert (sizetype, addr->elts[i].val);
+      part = fold_convert (int_type, addr->elts[i].val);
       if (addr->elts[i].coef != 1)
-	part = fold_build2 (MULT_EXPR, sizetype, part,
-			    wide_int_to_tree (sizetype, addr->elts[i].coef));
+	part = fold_build2 (MULT_EXPR, int_type, part,
+			    wide_int_to_tree (int_type, addr->elts[i].coef));
       add_to_parts (parts, part);
     }
   if (addr->rest)
-    add_to_parts (parts, fold_convert (sizetype, addr->rest));
+    add_to_parts (parts, fold_convert (int_type, addr->rest));
 }
 
 /* Force the PARTS to register.  */

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]