On 09/20/2016 11:11 AM, Segher Boessenkool wrote:
On Tue, Sep 20, 2016 at 08:52:46AM -0600, Jeff Law wrote:
- 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.


yes, see the comment before the declaration of
rtx_jump_insn::jump_label ()
it can be either a possibly deleted label, or a RETURN or SIMPLE_RETURN
expression.  Memory usage concerns aside its a tempting design to
change, but at the moment itts definitely intended to work this way,
there's plenty of code that checks ANY_RETURN_P (JUMP_LABEL (x)).

Yes.  But rtl.texi still says:

 Return insns count as jumps, but since they do not refer to any
 labels, their @code{JUMP_LABEL} is @code{NULL_RTX}.

And the comment at the top of jump.c:

  Each CODE_LABEL has a count of the times it is used
  stored in the LABEL_NUSES internal field, and each JUMP_INSN
  has one label that it refers to stored in the
  JUMP_LABEL internal field.  With this we can detect labels that
  become unused because of the deletion of all the jumps that
  formerly used them.  The JUMP_LABEL info is sometimes looked
  at by later passes.  For return insns, it contains either a
  RETURN or a SIMPLE_RETURN rtx.

It's intentional, and really there's nothing wrong with it, or with
operands[] containing CODE_LABELs. The problem is trying to force a

I'm just pointing out the documentation is out-of-date here.  I'll do
a patch.

static type system onto a dynamically typed data structure.
But what this doesn't answer is why we stuff RETURN/SIMPLE_RETURN into
the JUMP_LABEL field of a return.  I simply can't remember any rationale
behind that.

It is very inconvenient to parse the whole rtx to see if this is a
RETURN vs. a SIMPLE_RETURN (it can be inside a conditional or inside
a PARALLEL, etc.)
So it's similar to how we use JUMP_LABEL to find the jump target without having to dig through the whole rtx.



Ideally we would not have RETURN at all anymore, just SIMPLE_RETURN,
but that isn't going to happen any time soon.
And it wouldn't totally resolve this anyway.


I suspect that if we dig further, we'll find that we can accomplish
whatever the goal was behind that decision in a way that easily works in
a world where we have separated rtx objects from objects that are part
of the insn chain.

We cannot easily extract the JUMP_LABEL of a normal jump from its rtx
pattern either (if at all).  So it seems the extra field of a JUMP_INSN
is here to stay.  We'll have to figure out how to nicely do INSN vs.
JUMP_INSN in an rtx_insn world.
Both INSN and JUMP_INSNs are rtx_insn *s.

The problem is we're stuffing a (return) or (simple_return) rtx into a field that most of the time has an rtx_insn * (CODE_LABEL).

A hack-ish way to address this would be to create special CODE_LABELs that represent (return) and (simple_return) and stuff that into the JUMP_LABEL field. At which point the JUMP_LABEL field should turn into an rtx_insn * and the as_a casts related to this wart go away.

There may be cleaner ways, but there's certainly ways to move forward here.


Just to be clear, I don't see us going to a world where everything we
have as an rtx today is a statically typed object. But there are things
that I think make sense to carve out, the most obvious being objects
which are part of the insn chain.

Agreed on both.  I have nightmares about the first, so please don't even
talk about going there :-)
While I could see some value in static typing enough to allow static checking things like we don't try to access out-of-range operands (say XEXP (x, 1)) where X is a unary code. I don't see the level of pain to get there being worth it.

It's much like what we see in the tree world. There's value in separating _DECL, _TYPE and _EXPR nodes. But going deeper into in the _EXPR nodes seems like a huge level of pain.

jeff


Reply via email to