public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Harden rs6000 offsettable_ok_by_alignment
@ 2011-03-02  7:35 Alan Modra
  2011-03-07  0:01 ` David Edelsohn
  0 siblings, 1 reply; 6+ messages in thread
From: Alan Modra @ 2011-03-02  7:35 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Edelsohn

Prior to a fix I committed recently on the gcc-4.5-ibm branch, we used
to fail with a linker error: "relocation R_PPC64_TOC16_LO_DS not a
multiple of 4" on code loading the "md_backup_data-1" string in the
following testcase.

typedef __signed__ char __s8;
typedef unsigned char __u8;
typedef __signed__ short __s16;
typedef unsigned short __u16;
typedef __signed__ int __s32;
typedef unsigned int __u32;
typedef __signed__ long __s64;
typedef unsigned long __u64;

static struct mdp_backup_super {
 char magic[16];
 __u8 set_uuid[16];
 __u64 mtime;

 __u64 devstart;
 __u64 arraystart;
 __u64 length;
 __u32 sb_csum;
 __u32 pad1;
 __u64 devstart2;
 __u64 arraystart2;
 __u64 length2;
 __u32 sb_csum2;
 __u8 pad[512-68-32];
} __attribute__((aligned(512))) bsb;

void f (void)
{
  __builtin_memcpy (bsb.magic, "md_backup_data-1", 16);
}

The memcpy gets optimised to two ld insns followed by two std insns,
with both the address of bsb and the string being toc-relative.
bsb.magic is sufficiently aligned for the stores to be OK, but the
string is not aligned at all;  The loads may well be at an address
that is not a multiple of four.  It is also quite possible for the
string to straddle a 32k boundary, resulting in rubbish being loaded
in the second dword.

On mainline, the situation is a little different due to changes in
memref handling.  I seem to always get a VAR_DECL for the string,
correctly reporting an alignment of one, so we don't use the invalid
toc-relative address.  ie. I can't find a testcase for this potential
problem on mainline.  What worries me is that this may simply be due
to not trying enough compiler options.  Also, future changes in gcc
might result in STRING_CST trees appearing as they do on 4.5.  So I'd
like to apply the following patch.  Bootstrapped etc. powerpc64-linux.
OK?

	* config/rs6000/rs6000.c (offsettable_ok_by_alignment): Ensure
	STRING_CST returns false.  Add assert.

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 170373)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -7301,12 +7301,14 @@ offsettable_ok_by_alignment (tree decl)
   if (!decl)
     return true;
 
-  if (TREE_CODE (decl) != VAR_DECL
-      && TREE_CODE (decl) != PARM_DECL
-      && TREE_CODE (decl) != RESULT_DECL
-      && TREE_CODE (decl) != FIELD_DECL)
+  if (TREE_CODE (decl) == FUNCTION_DECL)
     return true;
 
+  if (CONSTANT_CLASS_P (decl))
+    return TREE_CODE (decl) != STRING_CST;
+
+  gcc_assert (DECL_P (decl));
+
   if (!DECL_SIZE_UNIT (decl))
     return false;
 

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Harden rs6000 offsettable_ok_by_alignment
  2011-03-02  7:35 Harden rs6000 offsettable_ok_by_alignment Alan Modra
@ 2011-03-07  0:01 ` David Edelsohn
  2011-03-07  3:41   ` Alan Modra
  2011-03-09  6:07   ` Fix pr48032, powerpc64 -mcmodel=medium invalid ld offset Alan Modra
  0 siblings, 2 replies; 6+ messages in thread
From: David Edelsohn @ 2011-03-07  0:01 UTC (permalink / raw)
  To: gcc-patches, Alan Modra

Isn't this too conservative?  Shouldn't CONSTANT_ALIGNMENT increase
the alignment of STRING_CST to word-aligned?  The only problem is
structs?

- David

On Wed, Mar 2, 2011 at 2:34 AM, Alan Modra <amodra@gmail.com> wrote:
> Prior to a fix I committed recently on the gcc-4.5-ibm branch, we used
> to fail with a linker error: "relocation R_PPC64_TOC16_LO_DS not a
> multiple of 4" on code loading the "md_backup_data-1" string in the
> following testcase.
>
> typedef __signed__ char __s8;
> typedef unsigned char __u8;
> typedef __signed__ short __s16;
> typedef unsigned short __u16;
> typedef __signed__ int __s32;
> typedef unsigned int __u32;
> typedef __signed__ long __s64;
> typedef unsigned long __u64;
>
> static struct mdp_backup_super {
>  char magic[16];
>  __u8 set_uuid[16];
>  __u64 mtime;
>
>  __u64 devstart;
>  __u64 arraystart;
>  __u64 length;
>  __u32 sb_csum;
>  __u32 pad1;
>  __u64 devstart2;
>  __u64 arraystart2;
>  __u64 length2;
>  __u32 sb_csum2;
>  __u8 pad[512-68-32];
> } __attribute__((aligned(512))) bsb;
>
> void f (void)
> {
>  __builtin_memcpy (bsb.magic, "md_backup_data-1", 16);
> }
>
> The memcpy gets optimised to two ld insns followed by two std insns,
> with both the address of bsb and the string being toc-relative.
> bsb.magic is sufficiently aligned for the stores to be OK, but the
> string is not aligned at all;  The loads may well be at an address
> that is not a multiple of four.  It is also quite possible for the
> string to straddle a 32k boundary, resulting in rubbish being loaded
> in the second dword.
>
> On mainline, the situation is a little different due to changes in
> memref handling.  I seem to always get a VAR_DECL for the string,
> correctly reporting an alignment of one, so we don't use the invalid
> toc-relative address.  ie. I can't find a testcase for this potential
> problem on mainline.  What worries me is that this may simply be due
> to not trying enough compiler options.  Also, future changes in gcc
> might result in STRING_CST trees appearing as they do on 4.5.  So I'd
> like to apply the following patch.  Bootstrapped etc. powerpc64-linux.
> OK?
>
>        * config/rs6000/rs6000.c (offsettable_ok_by_alignment): Ensure
>        STRING_CST returns false.  Add assert.
>
> Index: gcc/config/rs6000/rs6000.c
> ===================================================================
> --- gcc/config/rs6000/rs6000.c  (revision 170373)
> +++ gcc/config/rs6000/rs6000.c  (working copy)
> @@ -7301,12 +7301,14 @@ offsettable_ok_by_alignment (tree decl)
>   if (!decl)
>     return true;
>
> -  if (TREE_CODE (decl) != VAR_DECL
> -      && TREE_CODE (decl) != PARM_DECL
> -      && TREE_CODE (decl) != RESULT_DECL
> -      && TREE_CODE (decl) != FIELD_DECL)
> +  if (TREE_CODE (decl) == FUNCTION_DECL)
>     return true;
>
> +  if (CONSTANT_CLASS_P (decl))
> +    return TREE_CODE (decl) != STRING_CST;
> +
> +  gcc_assert (DECL_P (decl));
> +
>   if (!DECL_SIZE_UNIT (decl))
>     return false;
>
>
> --
> Alan Modra
> Australia Development Lab, IBM
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Harden rs6000 offsettable_ok_by_alignment
  2011-03-07  0:01 ` David Edelsohn
@ 2011-03-07  3:41   ` Alan Modra
  2011-03-09  6:07   ` Fix pr48032, powerpc64 -mcmodel=medium invalid ld offset Alan Modra
  1 sibling, 0 replies; 6+ messages in thread
From: Alan Modra @ 2011-03-07  3:41 UTC (permalink / raw)
  To: David Edelsohn; +Cc: gcc-patches

On Sun, Mar 06, 2011 at 07:01:44PM -0500, David Edelsohn wrote:
> Isn't this too conservative?  Shouldn't CONSTANT_ALIGNMENT increase
> the alignment of STRING_CST to word-aligned?

Yes and yes.  Thanks for the correction.  I'll update the patch.

> The only problem is structs?

No, I don't think there is anything special about the struct, just
that it was aligned more that usual.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Fix pr48032, powerpc64 -mcmodel=medium invalid ld offset
  2011-03-07  0:01 ` David Edelsohn
  2011-03-07  3:41   ` Alan Modra
@ 2011-03-09  6:07   ` Alan Modra
  2011-03-09 20:16     ` David Edelsohn
  1 sibling, 1 reply; 6+ messages in thread
From: Alan Modra @ 2011-03-09  6:07 UTC (permalink / raw)
  To: David Edelsohn; +Cc: gcc-patches

This patch
a) Moves the offsettable_ok_by_alignment call from rs6000_emit_move to
   legitimate_constant_pool_address_p, and
b) teaches offsettable_ok_by_alignment how to handle -fsection-anchors
   addresses, and
c) teaches offsettable_ok_by_alignment about other SYMBOL_REF_DECL
   trees I see there, various constant trees and CONSTRUCTOR.

About (a), well, that's just the way I should have written the
cmodel=medium optimization in the first place.  There is no alignment
reason to not create a cmodel=medium address (ie. addis,addi relative
to toc pointer), it's just that they do need to be sufficiently
aligned to use in a MEM.  We want reg=cmodel_medium_losum; mem[reg]
whenever we can do so, rather than creating a toc entry, but must not
allow the sequence to be combined into mem[cmodel_medium_losum] if
cmodel_medium_losum is not offsettable to access all the bytes of mem.
Perhaps legitimate_constant_pool_address_p should be renamed.  I
didn't do that because it already allows the non-constant-pool
cmodel=medium addresses, and the name is long enough now.  I commented
the function instead.

(b) is necessary because -fsection-anchors unfortunately loses the
real SYMBOL_REF_DECL when substituting anchor+offset into MEMs.  The
way I imlemented this is why legitimate_constant_pool_address_p needs
the MEM mode.  I suppose it would be possible to keep the original
SYMBOL_REF_DECL around in the rtl by some means or find the decl in
the struct object_block info, but both of these options seem like
overkill to me.  Note that I pass QImode to l_c_p_a_p from
rs6000_mode_dependent_address, print_operand_address, and the 'R'
constraint to indicate that any cmodel=medium address is permissable.

(c) was developed specifically to fix problems found on
ibm/gcc-4_5-branch.  I'd still like to commit this on mainline even
though it seems that mainline creates VAR_DECLs for constants.

Bootstrapped and regression tested powerpc64-linux.  OK to apply?

	PR target/48032
	* config/rs6000/rs6000.c (offsettable_ok_by_alignment): Do not
	presume symbol_refs without a symbol_ref_decl are suitably
	aligned, nor other trees we may see here.  Handle anchor symbols.
	(legitimate_constant_pool_address_p): Comment.  Add mode param.
	Check cmodel=medium addresses.  Adjust all calls.
	(rs6000_emit_move): Don't call offsettable_ok_by_alignment on
	creating cmodel=medium optimized access to locals.
	* config/rs6000/constraints.md (R): Pass QImode to
	legitimate_constant_pool_address_p.
	* config/rs6000/predicates.md (input_operand): Pass mode to
	legitimate_constant_pool_address_p.
	* config/rs6000/rs6000-protos.h (legitimate_constant_pool_address_p):
	Update prototype.

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 170807)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -5768,6 +5768,91 @@ virtual_stack_registers_memory_p (rtx op
 	  && regnum <= LAST_VIRTUAL_POINTER_REGISTER);
 }
 
+/* Return true if memory accesses to OP are known to never straddle
+   a 32k boundary.  */
+
+static bool
+offsettable_ok_by_alignment (rtx op, HOST_WIDE_INT offset,
+			     enum machine_mode mode)
+{
+  tree decl, type;
+  unsigned HOST_WIDE_INT dsize, dalign;
+
+  if (GET_CODE (op) != SYMBOL_REF)
+    return false;
+
+  decl = SYMBOL_REF_DECL (op);
+  if (!decl)
+    {
+      /* -fsection-anchors loses the original SYMBOL_REF_DECL when
+	 replacing memory addresses with an anchor plus offset.  We
+	 could find the decl by rummaging around in the block->objects
+	 VEC for the given offset but that seems like too much work.  */
+      dalign = 1;
+      if (SYMBOL_REF_HAS_BLOCK_INFO_P (op)
+	  && SYMBOL_REF_ANCHOR_P (op)
+	  && SYMBOL_REF_BLOCK (op) != NULL)
+	{
+	  struct object_block *block = SYMBOL_REF_BLOCK (op);
+	  HOST_WIDE_INT lsb, mask;
+
+	  /* Given the alignment of the block..  */
+	  dalign = block->alignment;
+	  mask = dalign / BITS_PER_UNIT - 1;
+
+	  /* ..and the combined offset of the anchor and any offset
+	     to this block object..  */
+	  offset += SYMBOL_REF_BLOCK_OFFSET (op);
+	  lsb = offset & -offset;
+
+	  /* ..find how many bits of the alignment we know for the
+	     object.  */
+	  mask &= lsb - 1;
+	  dalign = mask + 1;
+	}
+      return dalign >= GET_MODE_SIZE (mode);
+    }
+
+  if (DECL_P (decl))
+    {
+      if (TREE_CODE (decl) == FUNCTION_DECL)
+	return true;
+
+      if (!DECL_SIZE_UNIT (decl))
+	return false;
+
+      if (!host_integerp (DECL_SIZE_UNIT (decl), 1))
+	return false;
+
+      dsize = tree_low_cst (DECL_SIZE_UNIT (decl), 1);
+      if (dsize > 32768)
+	return false;
+
+      dalign = DECL_ALIGN_UNIT (decl);
+      return dalign >= dsize;
+    }
+
+  type = TREE_TYPE (decl);
+
+  if (TREE_CODE (decl) == STRING_CST)
+    dsize = TREE_STRING_LENGTH (decl);
+  else if (TYPE_SIZE_UNIT (type)
+	   && host_integerp (TYPE_SIZE_UNIT (type), 1))
+    dsize = tree_low_cst (TYPE_SIZE_UNIT (type), 1);
+  else
+    return false;
+  if (dsize > 32768)
+    return false;
+
+  dalign = TYPE_ALIGN (type);
+  if (CONSTANT_CLASS_P (decl))
+    dalign = CONSTANT_ALIGNMENT (decl, dalign);
+  else
+    dalign = DATA_ALIGNMENT (decl, dalign);
+  dalign /= BITS_PER_UNIT;
+  return dalign >= dsize;
+}
+
 static bool
 constant_pool_expr_p (rtx op)
 {
@@ -5792,8 +5877,12 @@ toc_relative_expr_p (rtx op)
 	  && XINT (tocrel_base, 1) == UNSPEC_TOCREL);
 }
 
+/* Return true if X is a constant pool address, and also for cmodel=medium
+   if X is a toc-relative address known to be offsettable within MODE.  */
+
 bool
-legitimate_constant_pool_address_p (const_rtx x, bool strict)
+legitimate_constant_pool_address_p (const_rtx x, enum machine_mode mode,
+				    bool strict)
 {
   return (TARGET_TOC
 	  && (GET_CODE (x) == PLUS || GET_CODE (x) == LO_SUM)
@@ -5802,7 +5891,12 @@ legitimate_constant_pool_address_p (cons
 	      || ((TARGET_MINIMAL_TOC
 		   || TARGET_CMODEL != CMODEL_SMALL)
 		  && INT_REG_OK_FOR_BASE_P (XEXP (x, 0), strict)))
-	  && toc_relative_expr_p (XEXP (x, 1)));
+	  && toc_relative_expr_p (XEXP (x, 1))
+	  && (TARGET_CMODEL != CMODEL_MEDIUM
+	      || constant_pool_expr_p (XVECEXP (tocrel_base, 0, 0))
+	      || mode == QImode
+	      || offsettable_ok_by_alignment (XVECEXP (tocrel_base, 0, 0),
+					      INTVAL (tocrel_offset), mode)));
 }
 
 static bool
@@ -5830,7 +5924,7 @@ rs6000_legitimate_offset_address_p (enum
     return false;
   if (!reg_offset_addressing_ok_p (mode))
     return virtual_stack_registers_memory_p (x);
-  if (legitimate_constant_pool_address_p (x, strict))
+  if (legitimate_constant_pool_address_p (x, mode, strict))
     return true;
   if (GET_CODE (XEXP (x, 1)) != CONST_INT)
     return false;
@@ -6830,7 +6924,8 @@ rs6000_legitimate_address_p (enum machin
     return 1;
   if (reg_offset_p && legitimate_small_data_p (mode, x))
     return 1;
-  if (reg_offset_p && legitimate_constant_pool_address_p (x, reg_ok_strict))
+  if (reg_offset_p
+      && legitimate_constant_pool_address_p (x, mode, reg_ok_strict))
     return 1;
   /* If not REG_OK_STRICT (before reload) let pass any stack offset.  */
   if (! reg_ok_strict
@@ -6940,7 +7035,7 @@ rs6000_mode_dependent_address (const_rtx
     case LO_SUM:
       /* Anything in the constant pool is sufficiently aligned that
 	 all bytes have the same high part address.  */
-      return !legitimate_constant_pool_address_p (addr, false);
+      return !legitimate_constant_pool_address_p (addr, QImode, false);
 
     /* Auto-increment cases are now treated generically in recog.c.  */
     case PRE_MODIFY:
@@ -7304,53 +7399,21 @@ rs6000_eliminate_indexed_memrefs (rtx op
 
   if (GET_CODE (operands[0]) == MEM
       && GET_CODE (XEXP (operands[0], 0)) != REG
-      && ! legitimate_constant_pool_address_p (XEXP (operands[0], 0), false))
+      && ! legitimate_constant_pool_address_p (XEXP (operands[0], 0),
+					       GET_MODE (operands[0]), false))
     operands[0]
       = replace_equiv_address (operands[0],
 			       copy_addr_to_reg (XEXP (operands[0], 0)));
 
   if (GET_CODE (operands[1]) == MEM
       && GET_CODE (XEXP (operands[1], 0)) != REG
-      && ! legitimate_constant_pool_address_p (XEXP (operands[1], 0), false))
+      && ! legitimate_constant_pool_address_p (XEXP (operands[1], 0),
+					       GET_MODE (operands[1]), false))
     operands[1]
       = replace_equiv_address (operands[1],
 			       copy_addr_to_reg (XEXP (operands[1], 0)));
 }
 
-/* Return true if memory accesses to DECL are known to never straddle
-   a 32k boundary.  */
-
-static bool
-offsettable_ok_by_alignment (tree decl)
-{
-  unsigned HOST_WIDE_INT dsize, dalign;
-
-  /* Presume any compiler generated symbol_ref is suitably aligned.  */
-  if (!decl)
-    return true;
-
-  if (TREE_CODE (decl) != VAR_DECL
-      && TREE_CODE (decl) != PARM_DECL
-      && TREE_CODE (decl) != RESULT_DECL
-      && TREE_CODE (decl) != FIELD_DECL)
-    return true;
-
-  if (!DECL_SIZE_UNIT (decl))
-    return false;
-
-  if (!host_integerp (DECL_SIZE_UNIT (decl), 1))
-    return false;
-
-  dsize = tree_low_cst (DECL_SIZE_UNIT (decl), 1);
-  if (dsize <= 1)
-    return true;
-  if (dsize > 32768)
-    return false;
-
-  dalign = DECL_ALIGN_UNIT (decl);
-  return dalign >= dsize;
-}
-
 /* Emit a move from SOURCE to DEST in mode MODE.  */
 void
 rs6000_emit_move (rtx dest, rtx source, enum machine_mode mode)
@@ -7672,8 +7735,7 @@ rs6000_emit_move (rtx dest, rtx source, 
 	  || (TARGET_CMODEL == CMODEL_MEDIUM
 	      && GET_CODE (operands[1]) == SYMBOL_REF
 	      && !CONSTANT_POOL_ADDRESS_P (operands[1])
-	      && SYMBOL_REF_LOCAL_P (operands[1])
-	      && offsettable_ok_by_alignment (SYMBOL_REF_DECL (operands[1]))))
+	      && SYMBOL_REF_LOCAL_P (operands[1])))
 	{
 	  rtx reg = NULL_RTX;
 	  if (TARGET_CMODEL != CMODEL_SMALL)
@@ -7695,7 +7757,8 @@ rs6000_emit_move (rtx dest, rtx source, 
 		   || (GET_CODE (operands[0]) == REG
 		       && FP_REGNO_P (REGNO (operands[0]))))
 	       && GET_CODE (operands[1]) != HIGH
-	       && ! legitimate_constant_pool_address_p (operands[1], false)
+	       && ! legitimate_constant_pool_address_p (operands[1], mode,
+							false)
 	       && ! toc_relative_expr_p (operands[1])
 	       && (TARGET_CMODEL == CMODEL_SMALL
 		   || can_create_pseudo_p ()
@@ -16421,7 +16484,7 @@ print_operand_address (FILE *file, rtx x
       fprintf (file, ")(%s)", reg_names[ REGNO (XEXP (x, 0)) ]);
     }
 #endif
-  else if (legitimate_constant_pool_address_p (x, true))
+  else if (legitimate_constant_pool_address_p (x, QImode, true))
     {
       /* This hack along with a corresponding hack in
 	 rs6000_output_addr_const_extra arranges to output addends
Index: gcc/config/rs6000/constraints.md
===================================================================
--- gcc/config/rs6000/constraints.md	(revision 170807)
+++ gcc/config/rs6000/constraints.md	(working copy)
@@ -166,7 +166,7 @@ (define_address_constraint "a"
 
 (define_constraint "R"
   "AIX TOC entry"
-  (match_test "legitimate_constant_pool_address_p (op, false)"))
+  (match_test "legitimate_constant_pool_address_p (op, QImode, false)"))
 
 ;; General constraints
 
Index: gcc/config/rs6000/predicates.md
===================================================================
--- gcc/config/rs6000/predicates.md	(revision 170807)
+++ gcc/config/rs6000/predicates.md	(working copy)
@@ -848,7 +848,7 @@ (define_predicate "input_operand"
     return 1;
 
   /* A SYMBOL_REF referring to the TOC is valid.  */
-  if (legitimate_constant_pool_address_p (op, false))
+  if (legitimate_constant_pool_address_p (op, mode, false))
     return 1;
 
   /* A constant pool expression (relative to the TOC) is valid */
Index: gcc/config/rs6000/rs6000-protos.h
===================================================================
--- gcc/config/rs6000/rs6000-protos.h	(revision 170807)
+++ gcc/config/rs6000/rs6000-protos.h	(working copy)
@@ -41,7 +41,8 @@ extern int small_data_operand (rtx, enum
 extern bool toc_relative_expr_p (rtx);
 extern bool invalid_e500_subreg (rtx, enum machine_mode);
 extern void validate_condition_mode (enum rtx_code, enum machine_mode);
-extern bool legitimate_constant_pool_address_p (const_rtx, bool);
+extern bool legitimate_constant_pool_address_p (const_rtx, enum machine_mode,
+						bool);
 extern bool legitimate_indirect_address_p (rtx, int);
 extern bool legitimate_indexed_address_p (rtx, int);
 extern bool avoiding_indexed_address_p (enum machine_mode);

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Fix pr48032, powerpc64 -mcmodel=medium invalid ld offset
  2011-03-09  6:07   ` Fix pr48032, powerpc64 -mcmodel=medium invalid ld offset Alan Modra
@ 2011-03-09 20:16     ` David Edelsohn
  2011-03-15  2:19       ` Alan Modra
  0 siblings, 1 reply; 6+ messages in thread
From: David Edelsohn @ 2011-03-09 20:16 UTC (permalink / raw)
  To: gcc-patches; +Cc: Alan Modra

On Wed, Mar 9, 2011 at 1:07 AM, Alan Modra <amodra@gmail.com> wrote:
> This patch
> a) Moves the offsettable_ok_by_alignment call from rs6000_emit_move to
>   legitimate_constant_pool_address_p, and
> b) teaches offsettable_ok_by_alignment how to handle -fsection-anchors
>   addresses, and
> c) teaches offsettable_ok_by_alignment about other SYMBOL_REF_DECL
>   trees I see there, various constant trees and CONSTRUCTOR.
>
> About (a), well, that's just the way I should have written the
> cmodel=medium optimization in the first place.  There is no alignment
> reason to not create a cmodel=medium address (ie. addis,addi relative
> to toc pointer), it's just that they do need to be sufficiently
> aligned to use in a MEM.  We want reg=cmodel_medium_losum; mem[reg]
> whenever we can do so, rather than creating a toc entry, but must not
> allow the sequence to be combined into mem[cmodel_medium_losum] if
> cmodel_medium_losum is not offsettable to access all the bytes of mem.
> Perhaps legitimate_constant_pool_address_p should be renamed.  I
> didn't do that because it already allows the non-constant-pool
> cmodel=medium addresses, and the name is long enough now.  I commented
> the function instead.
>
> (b) is necessary because -fsection-anchors unfortunately loses the
> real SYMBOL_REF_DECL when substituting anchor+offset into MEMs.  The
> way I imlemented this is why legitimate_constant_pool_address_p needs
> the MEM mode.  I suppose it would be possible to keep the original
> SYMBOL_REF_DECL around in the rtl by some means or find the decl in
> the struct object_block info, but both of these options seem like
> overkill to me.  Note that I pass QImode to l_c_p_a_p from
> rs6000_mode_dependent_address, print_operand_address, and the 'R'
> constraint to indicate that any cmodel=medium address is permissable.
>
> (c) was developed specifically to fix problems found on
> ibm/gcc-4_5-branch.  I'd still like to commit this on mainline even
> though it seems that mainline creates VAR_DECLs for constants.
>
> Bootstrapped and regression tested powerpc64-linux.  OK to apply?

Alan,

I agree that this is a better approach.  You might want to ask Richard
Sandiford to look at the section anchors issue.

Thanks, David

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Fix pr48032, powerpc64 -mcmodel=medium invalid ld offset
  2011-03-09 20:16     ` David Edelsohn
@ 2011-03-15  2:19       ` Alan Modra
  0 siblings, 0 replies; 6+ messages in thread
From: Alan Modra @ 2011-03-15  2:19 UTC (permalink / raw)
  To: David Edelsohn; +Cc: gcc-patches

After some discussion with David and Richard, this is what I'm about
to commit.  Differs from the previous patch in returning false for
BLKmode (via GET_MODE_SIZE(mode)==0 test) in the !decl branch of
offsettable_ok_by_alignment.

	PR target/48032
	* config/rs6000/rs6000.c (offsettable_ok_by_alignment): Do not
	presume symbol_refs without a symbol_ref_decl are suitably
	aligned, nor other trees we may see here.  Handle anchor symbols.
	(legitimate_constant_pool_address_p): Comment.  Add mode param.
	Check cmodel=medium addresses.  Adjust all calls.
	(rs6000_emit_move): Don't call offsettable_ok_by_alignment on
	creating cmodel=medium optimized access to locals.
	* config/rs6000/constraints.md (R): Pass QImode to
	legitimate_constant_pool_address_p.
	* config/rs6000/predicates.md (input_operand): Pass mode to
	legitimate_constant_pool_address_p.
	* config/rs6000/rs6000-protos.h (legitimate_constant_pool_address_p):
	Update prototype.

Index: gcc/config/rs6000/constraints.md
===================================================================
--- gcc/config/rs6000/constraints.md	(revision 170975)
+++ gcc/config/rs6000/constraints.md	(working copy)
@@ -166,7 +166,7 @@ (define_address_constraint "a"
 
 (define_constraint "R"
   "AIX TOC entry"
-  (match_test "legitimate_constant_pool_address_p (op, false)"))
+  (match_test "legitimate_constant_pool_address_p (op, QImode, false)"))
 
 ;; General constraints
 
Index: gcc/config/rs6000/predicates.md
===================================================================
--- gcc/config/rs6000/predicates.md	(revision 170975)
+++ gcc/config/rs6000/predicates.md	(working copy)
@@ -854,7 +854,7 @@ (define_predicate "input_operand"
     return 1;
 
   /* A SYMBOL_REF referring to the TOC is valid.  */
-  if (legitimate_constant_pool_address_p (op, false))
+  if (legitimate_constant_pool_address_p (op, mode, false))
     return 1;
 
   /* A constant pool expression (relative to the TOC) is valid */
Index: gcc/config/rs6000/rs6000-protos.h
===================================================================
--- gcc/config/rs6000/rs6000-protos.h	(revision 170975)
+++ gcc/config/rs6000/rs6000-protos.h	(working copy)
@@ -41,7 +41,8 @@ extern int small_data_operand (rtx, enum
 extern bool toc_relative_expr_p (rtx);
 extern bool invalid_e500_subreg (rtx, enum machine_mode);
 extern void validate_condition_mode (enum rtx_code, enum machine_mode);
-extern bool legitimate_constant_pool_address_p (const_rtx, bool);
+extern bool legitimate_constant_pool_address_p (const_rtx, enum machine_mode,
+						bool);
 extern bool legitimate_indirect_address_p (rtx, int);
 extern bool legitimate_indexed_address_p (rtx, int);
 extern bool avoiding_indexed_address_p (enum machine_mode);
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 170975)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -5791,6 +5791,94 @@ virtual_stack_registers_memory_p (rtx op
 	  && regnum <= LAST_VIRTUAL_POINTER_REGISTER);
 }
 
+/* Return true if memory accesses to OP are known to never straddle
+   a 32k boundary.  */
+
+static bool
+offsettable_ok_by_alignment (rtx op, HOST_WIDE_INT offset,
+			     enum machine_mode mode)
+{
+  tree decl, type;
+  unsigned HOST_WIDE_INT dsize, dalign;
+
+  if (GET_CODE (op) != SYMBOL_REF)
+    return false;
+
+  decl = SYMBOL_REF_DECL (op);
+  if (!decl)
+    {
+      if (GET_MODE_SIZE (mode) == 0)
+	return false;
+
+      /* -fsection-anchors loses the original SYMBOL_REF_DECL when
+	 replacing memory addresses with an anchor plus offset.  We
+	 could find the decl by rummaging around in the block->objects
+	 VEC for the given offset but that seems like too much work.  */
+      dalign = 1;
+      if (SYMBOL_REF_HAS_BLOCK_INFO_P (op)
+	  && SYMBOL_REF_ANCHOR_P (op)
+	  && SYMBOL_REF_BLOCK (op) != NULL)
+	{
+	  struct object_block *block = SYMBOL_REF_BLOCK (op);
+	  HOST_WIDE_INT lsb, mask;
+
+	  /* Given the alignment of the block..  */
+	  dalign = block->alignment;
+	  mask = dalign / BITS_PER_UNIT - 1;
+
+	  /* ..and the combined offset of the anchor and any offset
+	     to this block object..  */
+	  offset += SYMBOL_REF_BLOCK_OFFSET (op);
+	  lsb = offset & -offset;
+
+	  /* ..find how many bits of the alignment we know for the
+	     object.  */
+	  mask &= lsb - 1;
+	  dalign = mask + 1;
+	}
+      return dalign >= GET_MODE_SIZE (mode);
+    }
+
+  if (DECL_P (decl))
+    {
+      if (TREE_CODE (decl) == FUNCTION_DECL)
+	return true;
+
+      if (!DECL_SIZE_UNIT (decl))
+	return false;
+
+      if (!host_integerp (DECL_SIZE_UNIT (decl), 1))
+	return false;
+
+      dsize = tree_low_cst (DECL_SIZE_UNIT (decl), 1);
+      if (dsize > 32768)
+	return false;
+
+      dalign = DECL_ALIGN_UNIT (decl);
+      return dalign >= dsize;
+    }
+
+  type = TREE_TYPE (decl);
+
+  if (TREE_CODE (decl) == STRING_CST)
+    dsize = TREE_STRING_LENGTH (decl);
+  else if (TYPE_SIZE_UNIT (type)
+	   && host_integerp (TYPE_SIZE_UNIT (type), 1))
+    dsize = tree_low_cst (TYPE_SIZE_UNIT (type), 1);
+  else
+    return false;
+  if (dsize > 32768)
+    return false;
+
+  dalign = TYPE_ALIGN (type);
+  if (CONSTANT_CLASS_P (decl))
+    dalign = CONSTANT_ALIGNMENT (decl, dalign);
+  else
+    dalign = DATA_ALIGNMENT (decl, dalign);
+  dalign /= BITS_PER_UNIT;
+  return dalign >= dsize;
+}
+
 static bool
 constant_pool_expr_p (rtx op)
 {
@@ -5815,8 +5903,12 @@ toc_relative_expr_p (rtx op)
 	  && XINT (tocrel_base, 1) == UNSPEC_TOCREL);
 }
 
+/* Return true if X is a constant pool address, and also for cmodel=medium
+   if X is a toc-relative address known to be offsettable within MODE.  */
+
 bool
-legitimate_constant_pool_address_p (const_rtx x, bool strict)
+legitimate_constant_pool_address_p (const_rtx x, enum machine_mode mode,
+				    bool strict)
 {
   return (TARGET_TOC
 	  && (GET_CODE (x) == PLUS || GET_CODE (x) == LO_SUM)
@@ -5825,7 +5917,12 @@ legitimate_constant_pool_address_p (cons
 	      || ((TARGET_MINIMAL_TOC
 		   || TARGET_CMODEL != CMODEL_SMALL)
 		  && INT_REG_OK_FOR_BASE_P (XEXP (x, 0), strict)))
-	  && toc_relative_expr_p (XEXP (x, 1)));
+	  && toc_relative_expr_p (XEXP (x, 1))
+	  && (TARGET_CMODEL != CMODEL_MEDIUM
+	      || constant_pool_expr_p (XVECEXP (tocrel_base, 0, 0))
+	      || mode == QImode
+	      || offsettable_ok_by_alignment (XVECEXP (tocrel_base, 0, 0),
+					      INTVAL (tocrel_offset), mode)));
 }
 
 static bool
@@ -5853,7 +5950,7 @@ rs6000_legitimate_offset_address_p (enum
     return false;
   if (!reg_offset_addressing_ok_p (mode))
     return virtual_stack_registers_memory_p (x);
-  if (legitimate_constant_pool_address_p (x, strict))
+  if (legitimate_constant_pool_address_p (x, mode, strict))
     return true;
   if (GET_CODE (XEXP (x, 1)) != CONST_INT)
     return false;
@@ -6853,7 +6950,8 @@ rs6000_legitimate_address_p (enum machin
     return 1;
   if (reg_offset_p && legitimate_small_data_p (mode, x))
     return 1;
-  if (reg_offset_p && legitimate_constant_pool_address_p (x, reg_ok_strict))
+  if (reg_offset_p
+      && legitimate_constant_pool_address_p (x, mode, reg_ok_strict))
     return 1;
   /* If not REG_OK_STRICT (before reload) let pass any stack offset.  */
   if (! reg_ok_strict
@@ -6963,7 +7061,7 @@ rs6000_mode_dependent_address (const_rtx
     case LO_SUM:
       /* Anything in the constant pool is sufficiently aligned that
 	 all bytes have the same high part address.  */
-      return !legitimate_constant_pool_address_p (addr, false);
+      return !legitimate_constant_pool_address_p (addr, QImode, false);
 
     /* Auto-increment cases are now treated generically in recog.c.  */
     case PRE_MODIFY:
@@ -7327,53 +7425,21 @@ rs6000_eliminate_indexed_memrefs (rtx op
 
   if (GET_CODE (operands[0]) == MEM
       && GET_CODE (XEXP (operands[0], 0)) != REG
-      && ! legitimate_constant_pool_address_p (XEXP (operands[0], 0), false))
+      && ! legitimate_constant_pool_address_p (XEXP (operands[0], 0),
+					       GET_MODE (operands[0]), false))
     operands[0]
       = replace_equiv_address (operands[0],
 			       copy_addr_to_reg (XEXP (operands[0], 0)));
 
   if (GET_CODE (operands[1]) == MEM
       && GET_CODE (XEXP (operands[1], 0)) != REG
-      && ! legitimate_constant_pool_address_p (XEXP (operands[1], 0), false))
+      && ! legitimate_constant_pool_address_p (XEXP (operands[1], 0),
+					       GET_MODE (operands[1]), false))
     operands[1]
       = replace_equiv_address (operands[1],
 			       copy_addr_to_reg (XEXP (operands[1], 0)));
 }
 
-/* Return true if memory accesses to DECL are known to never straddle
-   a 32k boundary.  */
-
-static bool
-offsettable_ok_by_alignment (tree decl)
-{
-  unsigned HOST_WIDE_INT dsize, dalign;
-
-  /* Presume any compiler generated symbol_ref is suitably aligned.  */
-  if (!decl)
-    return true;
-
-  if (TREE_CODE (decl) != VAR_DECL
-      && TREE_CODE (decl) != PARM_DECL
-      && TREE_CODE (decl) != RESULT_DECL
-      && TREE_CODE (decl) != FIELD_DECL)
-    return true;
-
-  if (!DECL_SIZE_UNIT (decl))
-    return false;
-
-  if (!host_integerp (DECL_SIZE_UNIT (decl), 1))
-    return false;
-
-  dsize = tree_low_cst (DECL_SIZE_UNIT (decl), 1);
-  if (dsize <= 1)
-    return true;
-  if (dsize > 32768)
-    return false;
-
-  dalign = DECL_ALIGN_UNIT (decl);
-  return dalign >= dsize;
-}
-
 /* Emit a move from SOURCE to DEST in mode MODE.  */
 void
 rs6000_emit_move (rtx dest, rtx source, enum machine_mode mode)
@@ -7695,8 +7761,7 @@ rs6000_emit_move (rtx dest, rtx source, 
 	  || (TARGET_CMODEL == CMODEL_MEDIUM
 	      && GET_CODE (operands[1]) == SYMBOL_REF
 	      && !CONSTANT_POOL_ADDRESS_P (operands[1])
-	      && SYMBOL_REF_LOCAL_P (operands[1])
-	      && offsettable_ok_by_alignment (SYMBOL_REF_DECL (operands[1]))))
+	      && SYMBOL_REF_LOCAL_P (operands[1])))
 	{
 	  rtx reg = NULL_RTX;
 	  if (TARGET_CMODEL != CMODEL_SMALL)
@@ -7718,7 +7783,8 @@ rs6000_emit_move (rtx dest, rtx source, 
 		   || (GET_CODE (operands[0]) == REG
 		       && FP_REGNO_P (REGNO (operands[0]))))
 	       && GET_CODE (operands[1]) != HIGH
-	       && ! legitimate_constant_pool_address_p (operands[1], false)
+	       && ! legitimate_constant_pool_address_p (operands[1], mode,
+							false)
 	       && ! toc_relative_expr_p (operands[1])
 	       && (TARGET_CMODEL == CMODEL_SMALL
 		   || can_create_pseudo_p ()
@@ -16444,7 +16510,7 @@ print_operand_address (FILE *file, rtx x
       fprintf (file, ")(%s)", reg_names[ REGNO (XEXP (x, 0)) ]);
     }
 #endif
-  else if (legitimate_constant_pool_address_p (x, true))
+  else if (legitimate_constant_pool_address_p (x, QImode, true))
     {
       /* This hack along with a corresponding hack in
 	 rs6000_output_addr_const_extra arranges to output addends

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-03-15  2:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-02  7:35 Harden rs6000 offsettable_ok_by_alignment Alan Modra
2011-03-07  0:01 ` David Edelsohn
2011-03-07  3:41   ` Alan Modra
2011-03-09  6:07   ` Fix pr48032, powerpc64 -mcmodel=medium invalid ld offset Alan Modra
2011-03-09 20:16     ` David Edelsohn
2011-03-15  2:19       ` Alan Modra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).