> From: Richard Sandiford <rdsandif...@googlemail.com>
> Date: Sun, 22 Apr 2012 10:47:24 +0200
> With some trepidation, because I suspect I'm missing the point :-)
Maybe but maybe not. Below it seems my observation was
misdiagnosed, and this is just a minor bug.
> Hans-Peter Nilsson <hans-peter.nils...@axis.com> writes:
> > Other target-patches exposed this for me.
> > I have on the 4.7-branch an insn:
> >
> > (jump_insn 245 277 246 (set (pc)
> > (label_ref:SI 312)) whatever.c:511 -1
> > (nil)
> > -> 187)
> >
> > and two (or more) pattern candidates in the following .md file
> > order:
> >
> > (define_insn "jump"
> > [(set (pc)
> > (label_ref (match_operand 0 "" "")))]
> > ""
> > "ba %l0%#"
> > [(set_attr "slottable" "has_slot")])
> >
> > (define_insn "*indirect_jump_non_v32"
> > [(set (pc) (match_operand:SI 0 "nonimmediate_operand" "rm"))]
> > "!TARGET_V32"
> > "jump %0")
> [...]
> > It seems that since 4.3, some change now causes the generated
> > pattern-matching tree in insn-recog.c:recog to try the pattern
> > *with the specified mode* before (eventually, seemingly last)
> > the one with the unspecified-mode label_ref.
>
> This second one shouldn't match label_ref though, should it?
> label_ref is an immediate_operand.
My bad, bad example, but I thought this was beside the point.
My observation was a predicate unexpectedly called, not a
pattern actually wrongly matched.
> Was just wondering whether 4.3 was when the predicate codes
> thing was added, so that genrecog knew that nonimmediate_operand
> couldn't match.
And that's the reason the generated code calls it? :]
BTW, there's a second almost-matching pattern in cris.md with
register_operand as the predicate that I didn't show, but just
for clarity of the generated code:
(define_insn "*indirect_jump_v32"
[(set (pc) (match_operand:SI 0 "register_operand" "r"))]
"TARGET_V32"
"jump %0%#"
[(set_attr "slottable" "has_slot")])
The generated code in insn-recog.c has, seeing PC in XEXP (x0, 0)
n a SET, arriving at L1691:
L1691: ATTRIBUTE_UNUSED_LABEL
x1 = XEXP (x0, 1);
if (GET_MODE (x1) == SImode)
goto L2792;
L1687: ATTRIBUTE_UNUSED_LABEL
switch (GET_CODE (x1))
{
case LABEL_REF:
goto L1688;
case IF_THEN_ELSE:
goto L1701;
default:
break;
}
goto ret0;
L2792: ATTRIBUTE_UNUSED_LABEL
if (nonimmediate_operand (x1, SImode))
{
operands[0] = x1;
goto L1692;
}
L2793: ATTRIBUTE_UNUSED_LABEL
if (register_operand (x1, SImode))
{
operands[0] = x1;
goto L1696;
}
goto L1687;
Note how the mode and predicates are tested before checking for
LABEL_REF. But, as the predicates don't match, the last goto is
executed, going back to code checking for LABEL_REF. Verified
by stepping in gdb.
Oh, I see what you mean; I changed the predicate of the first
pattern to general_operand and now there's a check for LABEL_REF
before testing for general_operand! I also tried with a
target-local predicate, combinations both known and known not to
match LABEL_REF with and without specifying the mode. What
seems to throw genrecog.c off, is when there's a mode specified
on the candidate with the predicate ("*indirect_jump_non_v32"
above); it then tests the mode and the known-to-fail predicates
before going back to the candidate with the bare label_ref.
So that leaves us just a much smaller problem with suboptimal
insn-recog.c code (predicates known to fail are still called).
I'll open a PR and point to this message.
BTW, the mode on the label_ref is not often seen: the label_ref
has to be modified by middle-end (calls to gen_rtx_LABEL_REF, in
this case by redirect_target IIRC) and not coming from the
"jump" expander. Most targets still generated VOIDmode
LABEL_REFs in their jump patterns.
brgds, H-P
