Re: (patch,rfc) s/gimple/gimple */

[Trevor Saunders <tbsaunde@tbsaunde.org>]

On Mon, Sep 21, 2015 at 08:32:36PM +0100, Richard Sandiford wrote:
> Richard Biener <richard.guenther@gmail.com> writes:
> > On Fri, Sep 18, 2015 at 3:32 PM, Trevor Saunders <tbsaunde@tbsaunde.org> wrote:
> >> On Wed, Sep 16, 2015 at 03:11:14PM -0400, David Malcolm wrote:
> >>> On Wed, 2015-09-16 at 09:16 -0400, Trevor Saunders wrote:
> >>> > Hi,
> >>> >
> >>> > I gave changing from gimple to gimple * a shot last week.  It turned out
> >>> > to be not too hard.  As you might expect the patch is huge so its
> >>> > attached compressed.
> >>> >
> >>> > patch was bootstrapped + regtested on x86_64-linux-gnu, and run through
> >>> > config-list.mk.  However I needed to update it some for changes made
> >>> > while testing.  Do people want to make this change now?  If so I'll try
> >>> > and commit the patch over the weekend when less is changing.
> >>>
> >>>
> >>> FWIW there are some big changes in gcc/tree-vect-slp.c:vectorizable_load
> >>> that looks like unrelated whitespace changes, e.g. the following (and
> >>> there are some followup hunks).  Did something change underneath, or was
> >>> there a stray whitespace cleanup here?  (I skimmed through the patch,
> >>> and this was the only file I spotted where something looked wrong)
> >>
> >> yeah, it was a stray whitespace cleanup, but I reverted it.
> >>
> >> Given the few but only positive comments I've seen I'm planning to
> >> commit this over the weekend.
> >
> > Thanks a lot!
> >
> > If you are still in a refactoring mood then I have sth else here.  When
> > streamlining the gimple accessors I noticed the glaring const-correctness
> > issue in
> >
> > /* Return a pointer to the LHS of assignment statement GS.  */
> >
> > static inline tree *
> > gimple_assign_lhs_ptr (const gassign *gs)
> > {
> >   return const_cast<tree *> (&gs->op[0]);
> > }
> >
> > and was thinking to either "fix" it by removing the 'const' or by
> > merging gimple_assign_lhs and gimple_assign_lhs_ptr into
> >
> > static inline const tree&
> > gimple_assign_lhs (const gassign *);
> >
> > static inline tree&
> > gimple_assign_lhs (gassign *);
> 
> AIUI const_tree (like const_rtx) only protects the top-level tree.
> This is something I always hoped to change for rtl one day, but fixing
> all the fallout would be an incredibly dull task...
> 
> I suppose protecting the top level is still better than nothing though.

yeah, it seems like having things at least be standard const correct
here would be nice.  That said some of this looks interesting, it looks
like the use def data structures keep pointers to pointers to trees in
the middle of gimple objects.  That seems kind of odd, I guess the idea
is that that way sometimes the use def data structure will automatically
stay up to date?

Trev

> 
> Thanks,
> Richard
>