diff mbox

Fix PR tree-optimization/45722

Message ID 201011140017.47860.ebotcazou@adacore.com
State New
Headers show

Commit Message

Eric Botcazou Nov. 13, 2010, 11:17 p.m. UTC 
Hi,

this is the failure of gcc.c-torture/execute/20040709-2.c on various strict 
alignment platforms.  The SRA pass builds naked MEM_REFs to scalarize direct 
assignments between structures and this can run afoul of the old quirks of 
the indirect access support on strict alignment platforms.  So the attached 
patch arranges to always preserve existing COMPONENT_REF accesses around the 
MEM_REFs; I think a complete fix would be not to build the MEM_REFs in the 
first place as this introduces artificial addressability issues that aren't in 
the original code.  This in turn requires a small adjustment to the pattern 
matching code in ipa-prop.c.

Tested on x86-64/Linux, SPARC 32-bit and 64-bit Solaris.  OK for mainline?


2010-11-13  Eric Botcazou  <ebotcazou@adacore.com>

	PR tree-optimization/45722
	* tree-sra.c (build_ref_for_model): Always build a COMPONENT_REF if
	this is a reference to a component.
	* ipa-prop.c (ipa_get_member_ptr_load_param): Accept COMPONENT_REF.
	(ipa_note_param_call): Adjust comment.

Comments

Richard Biener Nov. 14, 2010, 10:30 a.m. UTC | #1 
On Sun, Nov 14, 2010 at 12:17 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> this is the failure of gcc.c-torture/execute/20040709-2.c on various strict
> alignment platforms.  The SRA pass builds naked MEM_REFs to scalarize direct
> assignments between structures and this can run afoul of the old quirks of
> the indirect access support on strict alignment platforms.  So the attached
> patch arranges to always preserve existing COMPONENT_REF accesses around the
> MEM_REFs; I think a complete fix would be not to build the MEM_REFs in the
> first place as this introduces artificial addressability issues that aren't in
> the original code.

What kind of artificial addressability issues?  Note that the base operand of
a MEM_REF does not have to be addressable.

> This in turn requires a small adjustment to the pattern
> matching code in ipa-prop.c.
>
> Tested on x86-64/Linux, SPARC 32-bit and 64-bit Solaris.  OK for mainline?

Ok.

Thanks,
Richard.

>
> 2010-11-13  Eric Botcazou  <ebotcazou@adacore.com>
>
>        PR tree-optimization/45722
>        * tree-sra.c (build_ref_for_model): Always build a COMPONENT_REF if
>        this is a reference to a component.
>        * ipa-prop.c (ipa_get_member_ptr_load_param): Accept COMPONENT_REF.
>        (ipa_note_param_call): Adjust comment.
>
>
> --
> Eric Botcazou
>
Eric Botcazou Nov. 14, 2010, 11:19 a.m. UTC | #2 
> What kind of artificial addressability issues?  Note that the base operand
> of a MEM_REF does not have to be addressable.

So the & in the first operand of the MEM_REF is purely formal?

> Ok.

Thanks.
Richard Biener Nov. 14, 2010, 11:27 a.m. UTC | #3 
On Sun, Nov 14, 2010 at 12:19 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> What kind of artificial addressability issues?  Note that the base operand
>> of a MEM_REF does not have to be addressable.
>
> So the & in the first operand of the MEM_REF is purely formal?

Yes.  It doesn't cause TREE_ADDRESSABLE to be set either (which means
it can expand to a register for example).

Richard.

>> Ok.
>
> Thanks.
>
> --
> Eric Botcazou
>
diff mbox

Patch

Index: tree-sra.c
===================================================================
--- tree-sra.c	(revision 166646)
+++ tree-sra.c	(working copy)
@@ -1391,7 +1391,7 @@  build_ref_for_offset (location_t loc, tr
 
 /* Construct a memory reference to a part of an aggregate BASE at the given
    OFFSET and of the same type as MODEL.  In case this is a reference to a
-   bit-field, the function will replicate the last component_ref of model's
+   component, the function will replicate the last COMPONENT_REF of model's
    expr to access it.  GSI and INSERT_AFTER have the same meaning as in
    build_ref_for_offset.  */
 
@@ -1400,12 +1400,9 @@  build_ref_for_model (location_t loc, tre
 		     struct access *model, gimple_stmt_iterator *gsi,
 		     bool insert_after)
 {
-  if (TREE_CODE (model->expr) == COMPONENT_REF
-      && DECL_BIT_FIELD (TREE_OPERAND (model->expr, 1)))
+  if (TREE_CODE (model->expr) == COMPONENT_REF)
     {
-      /* This access represents a bit-field.  */
       tree t, exp_type;
-
       offset -= int_bit_position (TREE_OPERAND (model->expr, 1));
       exp_type = TREE_TYPE (TREE_OPERAND (model->expr, 0));
       t = build_ref_for_offset (loc, base, offset, exp_type, gsi, insert_after);
Index: ipa-prop.c
===================================================================
--- ipa-prop.c	(revision 166646)
+++ ipa-prop.c	(working copy)
@@ -918,10 +918,15 @@  ipa_compute_jump_functions (struct cgrap
 static tree
 ipa_get_member_ptr_load_param (tree rhs, bool use_delta)
 {
-  tree rec, ref_offset, fld_offset;
-  tree ptr_field;
-  tree delta_field;
+  tree rec, ref_field, ref_offset, fld, fld_offset, ptr_field, delta_field;
 
+  if (TREE_CODE (rhs) == COMPONENT_REF)
+    {
+      ref_field = TREE_OPERAND (rhs, 1);
+      rhs = TREE_OPERAND (rhs, 0);
+    }
+  else
+    ref_field = NULL_TREE;
   if (TREE_CODE (rhs) != MEM_REF)
     return NULL_TREE;
   rec = TREE_OPERAND (rhs, 0);
@@ -933,6 +938,20 @@  ipa_get_member_ptr_load_param (tree rhs,
     return NULL_TREE;
 
   ref_offset = TREE_OPERAND (rhs, 1);
+
+  if (ref_field)
+    {
+      if (integer_nonzerop (ref_offset))
+	return NULL_TREE;
+
+      if (use_delta)
+	fld = delta_field;
+      else
+	fld = ptr_field;
+
+      return ref_field == fld ? rec : NULL_TREE;
+    }
+
   if (use_delta)
     fld_offset = byte_position (delta_field);
   else
@@ -1005,10 +1024,15 @@  ipa_note_param_call (struct cgraph_node
    below, the call is on the last line:
 
      <bb 2>:
+       f$__delta_5 = f.__delta;
+       f$__pfn_24 = f.__pfn;
+
+   or
+     <bb 2>:
        f$__delta_5 = MEM[(struct  *)&f];
        f$__pfn_24 = MEM[(struct  *)&f + 4B];
 
-     ...
+   and a few lines below:
 
      <bb 5>
        D.2496_3 = (int) f$__pfn_24;