[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