On 11/20/2017 06:41 AM, Aaron Sawdey wrote:
> 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.
Understood.

So what I think (and I'm hoping you can confirm one way or the other) is
that by generating this instruction you're turing a loop which
previously was considered a simple loop by the IV code and turning it
into something the IV bits no longer think is a simple loop.

I think that's problematical as when the loop is thought to be a simple
loop, it has to have a small number of forms for its loop back/exit loop
tests and whether or not a loop is a simple loop is cached in the loop
structure.

I think we need to dig into that first.  If my suspicion is correct then
this patch is really just papering over that deeper problem.  So I think
you need to dig a big deeper into why you're getting into the code in
question (canonicalize_condition) and whether or not the call chain
makes any sense given the changes you've made to the loop.


Jeff

Reply via email to