This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, PR 49094] Refrain from creating misaligned accesses in SRA
Hi,
On Thu, Jul 21, 2011 at 11:40:32AM +0200, Martin Jambor wrote:
> Hi,
>
> On Thu, Jul 21, 2011 at 10:34:35AM +0200, Richard Guenther wrote:
> > On Wed, 20 Jul 2011, Ulrich Weigand wrote:
> >
> > > Richard Guenther wrote:
> > > > On Tue, Jul 19, 2011 at 8:20 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> > > > > The problem is that in this expression
> > > > > disappear = VIEW_CONVERT_EXPR<struct VecClass>(x_8);
> > > > > the rhs is considered unaligned and blocks the SRA transformation.
> > > > >
> > > > > The check you added for SSA_NAMEs doesn't hit, because the SSA_NAME is
> > > > > encapsulated in a VIEW_CONVERT_EXPR. When get_object_alignment is called,
> > > > > the VIEW_CONVERT_EXPR is stripped off by get_inner_reference and the
> > > > > SSA_NAME appears, but then get_object_alignment doesn't handle it
> > > > > and just returns the default alignment of 8 bits.
> > > > >
> > > > > Maybe get_object_alignment should itself handle SSA_NAMEs?
> > > >
> > > > But what should it return for a rvalue? There is no "alignment" here.
> > > > I think SRA should avoid asking for rvalues.
> > >
> > > I must admit I do not fully understand what the SRA code is attempting
> > > to achieve here ... Could you elaborate on what you mean by "avoid
> > > asking for rvalues"? Should the SRA code never check the RHS of an
> > > assignment for alignment, only the LHS? Or should it classify the RHS
> > > tree according to whether the access is rvalue or lvalue (how would
> > > that work?)?
> >
> > Well, it should only ask for stores / loads. I'm not sure what we'd
> > want to return as alignment for an rvalue - MAX_ALIGNMENT? What should
> > we return for get_object_alignment of an INTEGER_CST for example?
> >
>
> Yeah, we certainly should not be examining alingment of invariants and
> of conversions of ssa names in. As far as rvalues in general are
> concerned, I don't really know which gimple predicate that would be.
> A comment suggests is_gimple_val but that does not return true for a
> VIEW_CONVERT_EXPR of an SSA_NAME and would return true for aggregate
> variables (which perhaps would not be a problem, however they do have
> an alignment).
>
> So at the moment I'd go for stripping all conversions from exp before
> the first if in tree_non_mode_aligned_mem_p and adding
> is_gimple_min_invariant to the condition. Does that make sense?
Like this? Ulrich, can you please verify it works? I have
bootstrapped this on x86_64 but there it obvioulsy works and my run of
compile/testsuite on compile farm sparc64 will take some time (plus,
the testcase you complained about passes there).
Thanks,
Martin
2011-07-25 Martin Jambor <mjambor@suse.cz>
* tree-sra.c (tree_non_mode_aligned_mem_p): Strip conversions and
return false for invariants.
Index: src/gcc/tree-sra.c
===================================================================
--- src.orig/gcc/tree-sra.c
+++ src/gcc/tree-sra.c
@@ -1075,9 +1075,14 @@ tree_non_mode_aligned_mem_p (tree exp)
enum machine_mode mode = TYPE_MODE (TREE_TYPE (exp));
unsigned int align;
+ while (CONVERT_EXPR_P (exp)
+ || TREE_CODE (exp) == VIEW_CONVERT_EXPR)
+ exp = TREE_OPERAND (exp, 0);
+
if (TREE_CODE (exp) == SSA_NAME
|| TREE_CODE (exp) == MEM_REF
|| mode == BLKmode
+ || is_gimple_min_invariant (exp)
|| !STRICT_ALIGNMENT)
return false;