New SPU failures (Re: [PATCH, PR 49923] Check for misaligned accesses before doing SRA)

Martin Jambor mjambor@suse.cz
Fri Sep 9 12:54:00 GMT 2011


Hi,

On Wed, Aug 10, 2011 at 04:29:40PM +0200, Richard Guenther wrote:
> On Wed, 10 Aug 2011, Ulrich Weigand wrote:
> 

...

> > 
> > FAIL: gcc.dg/ipa/ipa-sra-2.c scan-tree-dump eipa_sra "About to replace expr cow_.*D.->red with \\*ISRA"
> > FAIL: gcc.dg/ipa/ipa-sra-2.c scan-tree-dump eipa_sra "About to replace expr cow_.*D.->green with ISRA"
> > FAIL: gcc.dg/ipa/ipa-sra-2.c scan-tree-dump eipa_sra "About to replace expr calf_.*D.->red with \\*ISRA"
> > FAIL: gcc.dg/ipa/ipa-sra-2.c scan-tree-dump eipa_sra "About to replace expr calf_.*D.->green with ISRA"
> > FAIL: gcc.dg/ipa/ipa-sra-6.c scan-tree-dump-times eipa_sra "foo " 1

This has recvently been reported as PR 50052.

...

> > 
> > I must admin I continue to be confused about exactly what it is that
> > tree_non_mode_aligned_mem_p is supposed to be testing for.  We have:
> > 
> >   if (TREE_CODE (exp) == SSA_NAME
> >       || TREE_CODE (exp) == MEM_REF
> >       || mode == BLKmode
> >       || is_gimple_min_invariant (exp)
> >       || !STRICT_ALIGNMENT)
> >     return false;
> > 
> > So if we passed in the plain MEM_REF, this would be considered no problem.
> > The COMPONENT_REF does not add any additional misalignment, so one would
> > hope that this also shouldn't be a problem.
> > 
> > However, just because there *is* a COMPONENT_REF around it, we suddenly
> > realize the fact that don't have points-to information for the MEM_REF
> > and therefore consider *it* (and consequently the whole COMPONENT_REF)
> > to be potentially misaligned ...
> 
> Yep.  That's because we are totally confused about alignment :/
> 
> Martin, what we need to do is get expands idea of what alignment
> it woudl assume for a handled_component_ref and compare that
> to what it would say if we re-materialize the mem as a MEM_REF.
> 
> Unfortunately there isn't a function that you can use to mimic
> expands behavior (that of the normal_inner_ref: case), the closest
> would be to look for TYPE_PACKED or TYPE_ALIGN in addition to
> what get_object_alignment gives us.
> 
> Thus something like
> 
>   align = get_object_alignment (exp);
>   if (!TYPE_PACKED (TREE_TYPE (exp))
>       && (TREE_CODE (exp) != COMPONENT_REF
>           || !DECL_PACKED (TREE_OPERAND (exp, 1))))
>     align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), align);
>   if (GET_MODE_ALIGNMENT (mode) > align)
>     return true;
> 

This fixed the failing testcase ipa-sra-2.c but it creates a
misaligned access for the testcase below so I changed it to:

  align = get_object_alignment (exp);
  if (!TYPE_PACKED (TREE_TYPE (exp)))
    {
      bool encountered_packed = false;
      tree t = exp;

      while (TREE_CODE (t) == COMPONENT_REF)
        if (DECL_PACKED (TREE_OPERAND (t, 1)))
          {
            encountered_packed = true;
            break;
          }
        else
          t = TREE_OPERAND (t, 0);

      if (!encountered_packed)
        align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), align);
    }
  if (GET_MODE_ALIGNMENT (mode) > align)
    return true;

I'm currently bootstrapping this on a sparc64 on the compile farm.  Is
the patch OK if it passes (it has passed bootstrap and testing on
x86_64-linux but that is no surprise)?

Thanks,

Martin


2011-09-08  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/50052
	* tree-sra.c (tree_non_mode_aligned_mem_p): Look at TYPE_ALIGN if
	there is no packed field decl in exp.

	* gcc.dg/tree-ssa/pr49094-2.c: New test.

Index: src/gcc/testsuite/gcc.dg/tree-ssa/pr49094-2.c
===================================================================
--- /dev/null
+++ src/gcc/testsuite/gcc.dg/tree-ssa/pr49094-2.c
@@ -0,0 +1,44 @@
+/* { dg-do run } */
+/* { dg-options "-O" } */
+
+struct in_addr {
+  unsigned int s_addr;
+};
+
+struct ip {
+  struct in_addr ip_src,ip_dst;
+  unsigned char ip_p;
+  unsigned short ip_sum;
+};
+
+struct ip2 {
+  unsigned char blah;
+  unsigned short sheep;
+  struct ip ip;
+} __attribute__ ((aligned(1), packed));
+
+struct ip2 ip_fw_fwd_addr;
+
+int test_alignment( char *m )
+{
+  struct ip2 *ip = (struct ip2 *) m;
+  struct in_addr pkt_dst;
+  pkt_dst = ip->ip.ip_dst ;
+  if( pkt_dst.s_addr == 0 )
+    return 1;
+  else
+    return 0;
+}
+
+int __attribute__ ((noinline, noclone))
+intermediary (char *p)
+{
+  return test_alignment (p);
+}
+
+int
+main (int argc, char *argv[])
+{
+  ip_fw_fwd_addr.ip.ip_dst.s_addr = 1;
+  return intermediary ((void *) &ip_fw_fwd_addr);
+}
Index: src/gcc/tree-sra.c
===================================================================
--- src.orig/gcc/tree-sra.c
+++ src/gcc/tree-sra.c
@@ -1086,6 +1086,23 @@ tree_non_mode_aligned_mem_p (tree exp)
     return false;
 
   align = get_object_alignment (exp);
+  if (!TYPE_PACKED (TREE_TYPE (exp)))
+    {
+      bool encountered_packed = false;
+      tree t = exp;
+
+      while (TREE_CODE (t) == COMPONENT_REF)
+        if (DECL_PACKED (TREE_OPERAND (t, 1)))
+          {
+            encountered_packed = true;
+            break;
+          }
+        else
+          t = TREE_OPERAND (t, 0);
+
+      if (!encountered_packed)
+        align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), align);
+    }
   if (GET_MODE_ALIGNMENT (mode) > align)
     return true;
 



More information about the Gcc-patches mailing list