[PATCH, RFC]: Next stage1, refactoring: propagating rtx subclasses

Mikhail Maltsev maltsevm@gmail.com
Mon Apr 27 20:01:00 GMT 2015


I'm sending an updated patch (rebased to recent trunk, bootstrapped and
regtested on x86_64-unknown-linux-gnu).

On 04/25/2015 02:49 PM, Richard Sandiford wrote:
> FWIW I think the split between label_rtx and live_label_rtx is good,
> but I think we should give them different names.  The first one is
> returning only a position in the instruction stream, the second is
> returning a jump target.  I think we should rename both of them to
> make that distinction clearer.

I renamed live_label_rtx to jump_target_rtx. But I'm not sure if it is
appropriate (so, perhaps, you could give some advice about the right
names for these functions?)

> I think the eventual aim would be to have rtx_jump_insn member functions
> that get and set the jump label as an rtx_insn, with JUMP_LABEL_AS_INSN
> being a stepping stone towards that.  In cases like this it might make
> more sense to ensure old_jump has the right type (rtx_jump_insn) and go
> straight to the member functions, rather than switching to JUMP_LABEL_AS_INSN
> now and then having to rewrite it later.

I added the member functions. The problem is that JUMP_LABEL does not
always satisfy the current invariant of rtx_insn: it can also be an RTL
expression of type RETURN or SIMPLE_RETURN.

> Formatting nit, but the line break should be before "?" rather than after.
Fixed.

> This preserves the behaviour of the original code but I'm not sure
> it's worth it.  I doubt the distinction between:
> 
>   gcc_assert (JUMP_P (x));
> 
> and:
> 
>   gcc_checking_assert (JUMP_P (x));
> 
> was ever very scientific.  Seems like we should take this refactoring as
> an opportunity to make the checking more consistent.
Fixed (removed assert_as_a).

> That seems pretty heavy-weight for LRA-local code.  Also, the long-term
> plan is for INSN_LIST and rtx_insn to be in separate hierarchies,
> at which point we'd have no alias-safe way to distinguish them.
> 
> usage_insns isn't a GC structure and isn't live across a GC collection,
> so I don't think we need these lists to be rtxes at all.
OK, reverted changes in LRA code for now. I think this should be a
separate patch then.

-- 
Regards,
    Mikhail Maltsev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: as_insn2.patch
Type: text/x-patch
Size: 101097 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20150427/3f7f16f5/attachment.bin>


More information about the Gcc-patches mailing list