genmatch and cond vs "for (cnd cond vec_cond)" for gimple

genmatch and cond vs "for (cnd cond vec_cond)" for gimple

[Andrew Pinski]

Hi all,
  While moving the simple A CMP 0 ? A : -A patterns from
fold_cond_expr_with_comparison to match, I ran into an issue where
using cond directly in the patterns work while "for cnd (cond
vec_cond)" don't work.
It looks like in the first case we are able to correctly handle the
cond first operand being a comparison while with the for loop we are
not.

That is the following additional pattern works:
/* A == 0? A : -A    same as -A */
(simplify
 (cond (eq @0 zerop) @0 (negate@1 @0))
  (if (!HONOR_SIGNED_ZEROS (element_mode (type)))
   @1))
(simplify
 (cond (eq @0 zerop) zerop (negate@1 @0))
  (if (!HONOR_SIGNED_ZEROS (element_mode (type)))
   @1)))

While this one does not work:
(for cnd (cond vec_cond)
/* A == 0? A : -A    same as -A */
(simplify
 (cnd (eq @0 zerop) @0 (negate@1 @0))
  (if (!HONOR_SIGNED_ZEROS (element_mode (type)))
   @1))
(simplify
 (cnd (eq @0 zerop) zerop (negate@1 @0))
  (if (!HONOR_SIGNED_ZEROS (element_mode (type)))
   @1)))

---- CUT ---
I will try to debug genmatch some but I wanted to get this email out
to record what will need to be fixed to continue the movement of
phiopt over to match.

Thanks,
Andrew Pinski

Re: genmatch and cond vs "for (cnd cond vec_cond)" for gimple

[Andrew Pinski]

On Sat, Jun 12, 2021 at 4:54 PM Andrew Pinski <pinskia@gmail.com> wrote:
>
> Hi all,
>   While moving the simple A CMP 0 ? A : -A patterns from
> fold_cond_expr_with_comparison to match, I ran into an issue where
> using cond directly in the patterns work while "for cnd (cond
> vec_cond)" don't work.
> It looks like in the first case we are able to correctly handle the
> cond first operand being a comparison while with the for loop we are
> not.
>
> That is the following additional pattern works:
> /* A == 0? A : -A    same as -A */
> (simplify
>  (cond (eq @0 zerop) @0 (negate@1 @0))
>   (if (!HONOR_SIGNED_ZEROS (element_mode (type)))
>    @1))
> (simplify
>  (cond (eq @0 zerop) zerop (negate@1 @0))
>   (if (!HONOR_SIGNED_ZEROS (element_mode (type)))
>    @1)))
>
> While this one does not work:
> (for cnd (cond vec_cond)
> /* A == 0? A : -A    same as -A */
> (simplify
>  (cnd (eq @0 zerop) @0 (negate@1 @0))
>   (if (!HONOR_SIGNED_ZEROS (element_mode (type)))
>    @1))
> (simplify
>  (cnd (eq @0 zerop) zerop (negate@1 @0))
>   (if (!HONOR_SIGNED_ZEROS (element_mode (type)))
>    @1)))
>
> ---- CUT ---
> I will try to debug genmatch some but I wanted to get this email out
> to record what will need to be fixed to continue the movement of
> phiopt over to match.

So the problem is we lower for loops first and then cond.  Though
swapping the order in genmatch's lower function causes invalid C++
code to be generated :(.
Still trying to figure out why though.

Thanks,
Andrew

>
> Thanks,
> Andrew Pinski

Re: genmatch and cond vs "for (cnd cond vec_cond)" for gimple

[Andrew Pinski]

On Sat, Jun 12, 2021 at 5:21 PM Andrew Pinski <pinskia@gmail.com> wrote:
>
> On Sat, Jun 12, 2021 at 4:54 PM Andrew Pinski <pinskia@gmail.com> wrote:
> >
> > Hi all,
> >   While moving the simple A CMP 0 ? A : -A patterns from
> > fold_cond_expr_with_comparison to match, I ran into an issue where
> > using cond directly in the patterns work while "for cnd (cond
> > vec_cond)" don't work.
> > It looks like in the first case we are able to correctly handle the
> > cond first operand being a comparison while with the for loop we are
> > not.
> >
> > That is the following additional pattern works:
> > /* A == 0? A : -A    same as -A */
> > (simplify
> >  (cond (eq @0 zerop) @0 (negate@1 @0))
> >   (if (!HONOR_SIGNED_ZEROS (element_mode (type)))
> >    @1))
> > (simplify
> >  (cond (eq @0 zerop) zerop (negate@1 @0))
> >   (if (!HONOR_SIGNED_ZEROS (element_mode (type)))
> >    @1)))
> >
> > While this one does not work:
> > (for cnd (cond vec_cond)
> > /* A == 0? A : -A    same as -A */
> > (simplify
> >  (cnd (eq @0 zerop) @0 (negate@1 @0))
> >   (if (!HONOR_SIGNED_ZEROS (element_mode (type)))
> >    @1))
> > (simplify
> >  (cnd (eq @0 zerop) zerop (negate@1 @0))
> >   (if (!HONOR_SIGNED_ZEROS (element_mode (type)))
> >    @1)))
> >
> > ---- CUT ---
> > I will try to debug genmatch some but I wanted to get this email out
> > to record what will need to be fixed to continue the movement of
> > phiopt over to match.
>
> So the problem is we lower for loops first and then cond.  Though
> swapping the order in genmatch's lower function causes invalid C++
> code to be generated :(.
> Still trying to figure out why though.

I figured out why; lower_cond does not copy for_subst_vec for the new
simplifier.  Fixing that allows the switching of the order of the two
lower functions which is needed in this case.
I will submit the patch for this when I submit the patch set for
converting the simple "A CMP 0 ? A : - A" of
fold_cond_expr_with_comparison.

Thanks,
Andrew Pinski

>
> Thanks,
> Andrew
>
> >
> > Thanks,
> > Andrew Pinski

Re: genmatch and cond vs "for (cnd cond vec_cond)" for gimple

[Richard Biener]

On June 13, 2021 4:03:16 AM GMT+02:00, Andrew Pinski <pinskia@gmail.com> wrote:
>On Sat, Jun 12, 2021 at 5:21 PM Andrew Pinski <pinskia@gmail.com>
>wrote:
>>
>> On Sat, Jun 12, 2021 at 4:54 PM Andrew Pinski <pinskia@gmail.com>
>wrote:
>> >
>> > Hi all,
>> >   While moving the simple A CMP 0 ? A : -A patterns from
>> > fold_cond_expr_with_comparison to match, I ran into an issue where
>> > using cond directly in the patterns work while "for cnd (cond
>> > vec_cond)" don't work.
>> > It looks like in the first case we are able to correctly handle the
>> > cond first operand being a comparison while with the for loop we
>are
>> > not.
>> >
>> > That is the following additional pattern works:
>> > /* A == 0? A : -A    same as -A */
>> > (simplify
>> >  (cond (eq @0 zerop) @0 (negate@1 @0))
>> >   (if (!HONOR_SIGNED_ZEROS (element_mode (type)))
>> >    @1))
>> > (simplify
>> >  (cond (eq @0 zerop) zerop (negate@1 @0))
>> >   (if (!HONOR_SIGNED_ZEROS (element_mode (type)))
>> >    @1)))
>> >
>> > While this one does not work:
>> > (for cnd (cond vec_cond)
>> > /* A == 0? A : -A    same as -A */
>> > (simplify
>> >  (cnd (eq @0 zerop) @0 (negate@1 @0))
>> >   (if (!HONOR_SIGNED_ZEROS (element_mode (type)))
>> >    @1))
>> > (simplify
>> >  (cnd (eq @0 zerop) zerop (negate@1 @0))
>> >   (if (!HONOR_SIGNED_ZEROS (element_mode (type)))
>> >    @1)))
>> >
>> > ---- CUT ---
>> > I will try to debug genmatch some but I wanted to get this email
>out
>> > to record what will need to be fixed to continue the movement of
>> > phiopt over to match.
>>
>> So the problem is we lower for loops first and then cond.  Though
>> swapping the order in genmatch's lower function causes invalid C++
>> code to be generated :(.
>> Still trying to figure out why though.
>
>I figured out why; lower_cond does not copy for_subst_vec for the new
>simplifier.  Fixing that allows the switching of the order of the two
>lower functions which is needed in this case.
>I will submit the patch for this when I submit the patch set for
>converting the simple "A CMP 0 ? A : - A" of
>fold_cond_expr_with_comparison.

Hmm, it was done so on purpose because cond lowering does not see what is a cond w/o for lowered. So why does it not work with lowered for? 

I'll have a look next week. 

Richard. 

>Thanks,
>Andrew Pinski
>
>>
>> Thanks,
>> Andrew
>>
>> >
>> > Thanks,
>> > Andrew Pinski