On Mon, Jul 25, 2011 at 7:52 PM, Martin Jambor <mjam...@suse.cz> wrote: > 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 <uweig...@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 <mjam...@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)
There can be no convert exprs here, and there can be at most one VIEW_CONVERT_EXPR. > + || 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; Otherwise ok. Thanks, Richard.
