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 > 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
