This is the mail archive of the gcc-patches@gcc.gnu.org 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: [PATCH 0/8] make next_*_insn take rtx_insn * arguments


On Thu, Sep 15, 2016 at 10:27:56AM -0600, Jeff Law wrote:
> On 09/15/2016 10:10 AM, Trevor Saunders wrote:
> > On Thu, Sep 15, 2016 at 12:52:59PM +0200, Bernd Schmidt wrote:
> > > On 09/14/2016 09:21 PM, tbsaunde+gcc@tbsaunde.org wrote:
> > > 
> > > > Basically $subject.  First change variable's type to rtx_insn * where possible.
> > > > Then change the functions and fixup callers where it is still necessary to
> > > > cast.
> > > 
> > > #2, #4 and #8 look good and can be applied if they work independently of the
> > > others.
> > 
> > at most #2 should depend on #1 so it should be fine and I can check on
> > the others.
> > 
> > > Less certain about some of the others which introduce additional casts.
> > 
> > yeah, its somewhat unfortunate, though one way or another we will need
> > to add casts I think the question is just how many we will accept and
> > where.
> In my mind the casts represent the "bounds" of how far we've taken this
> work.  They occur at the boundaries where we haven't converted something
> from an "rtx" to an "rtx_insn *" and allow us to do the work piecemeal
> rather than all-at-once.
> 
> What I don't have a sense of is when we'll be able to push rtx_insn * far
> enough that we're able to start removing casts.
> 
> And that might be a good way to prioritize the next batch of work.  Pick a
> cast, remove it and deal with the fallout.  :-)
> 
> 
> > 
> > > Maybe LABEL_REF_LABEL needs converting first, and reorg.c has a few
> > > variables that might have to be made rtx_insn * in patch #7 to avoid casts.
> > 
> > LABEL_REF_LABEL might be doable, its a good idea I'll look into.  The
> > reorg.c stuff around target_label is rather complicated unfortunately.
> > In the end I of course agree the variables should be rtx_insn *.
> > However currently things are assigned to that variable that are not
> > insns.  So we need to break the variable up, but its involved in a lot
> > of code I don't think I know well enough to really refactor.  For
> > example it looks like target_label can hold a value between iterations
> > of the loop, I suspect that would be a bug, but I'm not really sure.
> I can probably help with reorg.  Hell, you might even be referring to my
> code!

ok, going through all the casts added here I see the following reasons
for them.

- in md files operands is a array of rtx, we should probably have a
  different way to pass insns to the C code here.  That seems worth
  investigating incrementally.

- JUMP_LABEL can be a return which is not an insn.  That also seems
  like something that should be improved at some point.

- tablejump_p returns a label through a rtx * out argument.  I expect
  that isn't hard to fix at this point.

- sh.c uses XEXP (SET_SRC (PATTERN (jump)) 0) with a comment JUMP_LABEL
  might be undefined when not optimizing.  Its not clear to me if that
  is still true.

- var_loc_node::loc is either an expr list or a note, off hand I'm not
  sure what to do with this.

- in reorg.c variables refer to both a insn and other things I think
  this is more results of JUMP_LABEL some times being a return.

I've locally converted LABEL_REF_LABEL, but that only avoids 2 casts.
However it does seem like an improvement so I'll send that shortly.

Trev

> jeff


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