This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [tree-ssa][PATCH] Let remove_stmt not always remoev annotations(was Re: [tree-ssa]: Code movement is a pain in the ass.)

On Mon, 11 Aug 2003, Andrew MacLeod wrote:

> On Mon, 2003-08-11 at 17:19, Daniel Berlin wrote:
> >
> >
> > On Mon, 11 Aug 2003, Andrew MacLeod wrote:
> >
> > > On Mon, 2003-08-11 at 16:39, Daniel Berlin wrote:
> > > >
> > > >
> > > > On Mon, 11 Aug 2003, Daniel Berlin wrote:
> > > >
> > >
> > > > Actually, doing it this way is going to make bsi_remove a do nothing
> > > > function that is only a single line wrapper for the new function. This
> > > > seems odd all not to just add a parameter.
> > > >
> > > I know, but when you are dealing with a client API, the second parameter
> > > is going to serve no purpose...
> >
> > Except there are other times you might want to do this, and then
> > you've got to go write another bsi fnuction for those too.
> Im not sure what you are getting at.  the only thing you are adding to
> the interface (well, going to add :-) is bsi_move() which happens to
> require something with almost the same code as bsi_remove, so you are
> commoning out the code for bsi_remove. The fact that you've commoned it
> all the way to calling a single routine with a flag seems like a good
> thing, not something to complain about.
> You seemed opposed to interface wrappers...  At least thats the way its
> coming across to me. You'd rather change the API and all its uses than
> have the API routine simply call another routine that has a different
> interface.

No, i'd rather the interface do only what it's documented to do, and if
you want it to do more, use a seperate function or a new argument.
Nowhere does it say "this will destroy more than the statement, it will
destroy things auxillary to the statement too".  Removal and destruction
aren't the same operation.  Having something labeled remove perform
destruction is bad.

> > This is a completely different thing that is not related to removing
> > a statement.
> > >Adding a flag to adjust internal only things merely
> > > obfuscates it. Anyone going to use it or read the code is going to have
> > > to go and lookup what the flag does.  Much cleaner to let bsi_remove
> > > simply have one parameter.  Plus then you dont go changing any client
> > > code, which is really the point to having an API in the first place.
> > >
> > Except when the API does nothing like what you would expect.  Would you
> > have guessed that bsi_remove, documented to remove a statement, would
> > modify your statement's SSA_NAME's as well?
> > I didn't.
> well your stmt doesnt exist any more... you are destroying it.

No, i'm *removing* it from the block stream.  This is not bsi_destroy
or bsi_delete.  This is a completely different operation.

>  I
> wouldn't expect to be able to look at it any more. Anyone else referring
> to it better not be able to refer to it either. It would never occur to
> me to look at the statement after removing it. its gone. I dont know why
> you'd care what it does to the stmt. As far as Im concerned it may have
> freed storage for it.
That would not be the job of bsi_remove, that wuold be the job of a
bsi_destroy or bsi_delete.

> >  > I also wouldn't call it bsi_remove_real, that seems a little  bit >
> > flippant :-).  Besides, it would require extra reading to figure out
> > > which one you need... :-)
> >
> > Other than the name, can i commit the patch?
> >
> Sure.
> Andrew

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]