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+...@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
ok, going through all the casts added here I see the following reasons
- 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
- 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.