On Sun, 2017-11-19 at 16:44 -0700, Jeff Law wrote:
> On 11/15/2017 08:40 AM, Aaron Sawdey wrote:
> > So, the story of this very small patch starts with me adding
> > patterns
> > for ppc instructions bdz[tf] and bdnz[tf] such as this:
> >
> > [(set (pc)
> > (if_then_else
> > (and
> > (ne (match_operand:P 1 "register_operand" "c,*b,*b,*b")
> > (const_int 1))
> > (match_operator 3 "branch_comparison_operator"
> > [(match_operand 4 "cc_reg_operand" "y,y,y,y")
> > (const_int 0)]))
> > (label_ref (match_operand 0))
> > (pc)))
> > (set (match_operand:P 2 "nonimmediate_operand"
> > "=1,*r,m,*d*wi*c*l")
> > (plus:P (match_dup 1)
> > (const_int -1)))
> > (clobber (match_scratch:P 5 "=X,X,&r,r"))
> > (clobber (match_scratch:CC 6 "=X,&y,&y,&y"))
> > (clobber (match_scratch:CCEQ 7 "=X,&y,&y,&y"))]
> >
> > However when this gets to the loop_doloop pass, we get an assert
> > fail
> > in iv_number_of_iterations():
> >
> > gcc_assert (COMPARISON_P (condition));
> >
> > This is happening because this branch insn tests two things ANDed
> > together so the and is at the top of the expression, not a
> > comparison.
>
> Is this something you've created for an existing loop? Presumably an
> existing loop that previously was a simple loop?
The rtl to use this instruction is generated by new code I'm working on
to do a builtin expansion of memcmp using a loop. I call
gen_bdnztf_di() to generate rtl for the insn. It would be nice to be
able to generate this instruction from doloop conversion but that is
beyond the scope of what I'm working on presently.
> >
> > This condition is extracted from the insn by get_condition() which
> > is
> > pretty straightforward, and which calls canonicalize_condition()
> > before
> > returning it. Now, one could put a test for a jump condition that
> > is
> > not a conditional test in here but the comment for
> > canonicalize_condition() says:
> >
> > (1) The code will always be a comparison operation (EQ, NE, GT,
> > etc.).
> >
> > So, this patch adds a test at the end that just returns 0 if the
> > return
> > rtx is not a comparison. As it happens, doloop conversion is not
> > needed
> > here because I'm already generating rtl for a branch-decrement
> > counter
> > based loop.
>
> I'm not at all familiar with this code, but I would probably guess
> that
> COND is supposed to appear within INSN. Canonicalize comparison is
> supposed to modify the COND expression that appears within INSN. THe
> overall structure of INSN doesn't much matter as long as COND refers
> to
> a comparison code with two operands.
>
> My gut tells me the problem is upstream or downstream in the call
> chain
> or that you've taken a loop that was considered a simple loop and
> modified the exit test in a way that other code is not prepared to
> handle.
Yes, get_condition() is the other place that this could be fixed. It
expects rtl of the form:
(set (pc) (if_then_else (cond ...
and here it's seeing something of the form:
(set (pc) (if_then_else (and (cond ... ) (cond ...)
It seems legitimate to add a check for this in get_condition() as well:
cond = XEXP (SET_SRC (set), 0);
if (!COMPARISON_P (cond))
return NULL_RTX;
There are a couple of uses of canonicalize_condition() in ifcvt.c and
it looks like the code there may expect the return to be a conditional.
This suggests it might be a good idea to do both checks.
Thanks,
Aaron
--
Aaron Sawdey, Ph.D. acsaw...@linux.vnet.ibm.com
050-2/C113 (507) 253-7520 home: 507/263-0782
IBM Linux Technology Center - PPC Toolchain
