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] |
On 09/16/2016 04:10 AM, Trevor Saunders wrote:
Agreed. Presumably the exception is passing around something like a CODE_LABEL? Or maybe some hackery with passing around a jump table?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.
The JUMP_LABEL field within a JUMP_INSN? That sounds like a bug.
- tablejump_p returns a label through a rtx * out argument. I expect that isn't hard to fix at this point.
Right. Seems like a reasonable follow-up.
The code in question is nearly 19 years old. What I think Joern was referring to here was that in the old days jump.c was responsible for setting JUMP_LABEL and that would only happen when optimizing.- 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.
JUMP_LABEL was a convenient way to find the jump target of an insn. It didn't matter where the label appeared as an operand. It was also the case that we could derive a label, even though the insn might have been an indirect jump.
Anyway...So instead of relying on JUMP_LABEL he extracts the destination out of the pattern (JUMP_LABEL is field outside the insn's pattern). That destination should be a LABEL_REF. Operand 0 of a LABEL_REF is its associated CODE_LABEL. In the sh.c case, he happens to know precisely where the LABEL_REF will be inside the insn's pattern.
This points out one of the little hairballs we're going to want to sort out. Essentially XEXP (X, 0) where X is a LABEL_REF needs to be an rtx_insn *. Right now it's an rtx.
- var_loc_node::loc is either an expr list or a note, off hand I'm not sure what to do with this.
Depending on how it's set we may just want to break this into two fields.
I'd probably want to sit down with a debugger or with a more detailed description of when this happens. I wouldn't be surprised if this was just a convenient way of marking jumps which we were later going to turn into simple returns.- 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.
Sounds good. jeff
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |