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]

Re: patch for mips flow problem, including test case


This is a generic problem with gcc.  We don't have any reliable way to
distinguish tablejumps from indirect jumps (resulting from computed gotos).

The addition of (use (label)) to tablejumps is an anachronism.  That was
necessary in the early days, but it has been unnecessary ever since the
REG_LABEL reg notes were added.  There are a number of ports nowadays that
don't bother to include the (use (label)) in tablejump patterns.  The MIPS
is one.  PA is another (see the casesi0 pattern).  There may be others.

If we are going to adopt that approach that a tablejump must include a
(use (label)), then it would be useful to document this, and verify that
all ports follow this convention.  Also, some code in the compiler could
be simplified.  There is some pretty ugly code in profile.c that exists
to recognize MIPS and HPPA tablejumps.  Look for the tablejump_entry_p
code.  This code could be deleted if this convention is followed.

There are also some optimization problems.  For instance, loop will not
optimize a loop that contains a switch statement, because it thinks the
tablejump entry instruction is an indirect branch that might exit the
loop.  See the REG_LABEL check in find_and_verify_loops.  We could ignore
a REG_LABEL if it appears on an insn with a (use (label)).  Or perhaps
not add a REG_LABEL note to such an instruction in the first place.

Alternatively, we could perhaps solve this problem by making flow smarter
about identifying tablejump entry instructions.  For instance, a tablejump
entry should always be followed by a ADDR_DIFF_VEC or ADDR_VEC, so if we
see a REG_LABEL note, check to see if the following insn is an ADDR*VEC.
This would be harder to do in profile.c though.

The patch is OK.  It doesn't completely fix the problem I just described,
but I don't see any harm to it, and it will fix the problem you reported,
so we may as well put it in.

Incidentally, the conditions in the tablejump_internal[34]+1 patterns exist so
that we won't accidentally create a tablejump insn by combining a indirect jump
with something else.  I suspect that these conditions would no longer be
necessary.  This isn't something that has to be fixed though, you could just
add a comment saying the conditions are probably obsolete.

Another incidentally, I see that there are still other tablejump patterns in
the mips.md file that are missing the (use (label)).  This includes at least
tablejump_mips161, tablejump_mips162, and casesi_internal.  Again, this isn't
something that has to be fixed.  It just means your patch doesn't fix the
entire problem.

Jim


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]