Re: Ping patch

[Richard Henderson <rth at redhat dot com>]

On Thu, Apr 21, 2005 at 09:56:52AM -0400, Daniel Berlin wrote:
> > val.alignment.offset = ((val.alignment.offset + BITS_PER_UNIT * newoffset)
> > 			% val.alignment.n);

> +	  val.alignment.offset += ((val.alignment.offset + 
> +				    BITS_PER_UNIT * newoffset) 
> +				   % val.alignment.n);

Still with the +=.

> > 
> > +  if (TREE_CODE (sym) == PARM_DECL 
> > +      || (decl_function_context (sym) != current_function_decl
> > +	  || TREE_STATIC (sym)))
> > 
> > I think this is confused.  All DECLs have their own alignment,
> > regardless of PARM_DECL or global or whatever.  Further, I think
> > you're confusing the alignment of &x with the alignment of x.
> > Certainly you're talking about both things here in the same if.
> 
> Only because i was told that pointer types have the minimum alignment
> required by their underlying type.
> IE that the alignment of &x is required to be at least the alignment of x.

And that is true, but that wasn't what you were computing.

> +static value
> +get_default_value (tree var)
> +{
> +  value val;
> +  val.lattice_val = KNOWN;
> +  val.alignment.n = TYPE_ALIGN (TREE_TYPE (var));
> +  val.alignment.offset = 0;
> +  return val;
> +}

This is also wrong.  You're computing the alignment of the *pointer*
rather than the alignment of the type pointed to.  So you get 64 for
"char *" instead of 8.

I'll also complain that you're not handling ADDR_EXPR anywhere, and
thus have no opportunity to look at DECL_ALIGN.  Which is *extremely*
important for the vectorizer.


r~