Re: Match-and-simplify and COND_EXPR

[pinskia at gmail dot com]

> On Nov 6, 2014, at 11:24 PM, Richard Biener <rguenther@suse.de> wrote:
> 
>> On November 7, 2014 5:03:19 AM CET, Andrew Pinski <pinskia@gmail.com> wrote:
>> On Thu, Nov 6, 2014 at 2:40 AM, Richard Biener <rguenther@suse.de>
>> wrote:
>>> On Thu, 6 Nov 2014, Richard Biener wrote:
>>> 
>>>>> On Wed, 5 Nov 2014, Andrew Pinski wrote:
>>>>> 
>>>>> Hi,
>>>>>  I was trying to hook up tree-ssa-phiopt to match-and-simplify
>> using
>>>>> either gimple_build (or rather using gimple_simplify depending on
>> if
>>>>> we want to produce cond_expr for conditional move).  I ran into a
>>>>> problem.
>>>>> With the pattern below:
>>>>> /* a ? 0 : 1 -> a if 0 and 1 are integral types. */
>>>>> (simplify
>>>>>  (cond_expr @0 integer_zerop integer_onep)
>>>>>  (if (INTEGRAL_TYPE_P (type))
>>>>>    (convert @0)))
>>>> 
>>>> Ok, so you are capturing a GENERIC expr here but nothing knows that.
>>>> It would work if you'd do (ugh)
>>>> 
>>>> (for op (lt le eq ne ge gt)
>>>> (simplify
>>>>  (cond_expr (op @0 @1) integer_zerop integer_onep)
>>>>  (if (INTEGRAL_TYPE_P (type))
>>>>   (convert (op @0 @1)))))
>>>> (simplify
>>>> (cond_expr SSA_NAME@0 integer_zerop integer_onep)
>>>>  (if (INTEGRAL_TYPE_P (type))
>>>>   (convert @0))))
>>>> 
>>>> as a workaround.  To make your version work will require (quite)
>>>> some special-casing in the code generator or maybe the resimplify
>>>> helper.  Let me see if I can cook up a "simple" fix.
>>> 
>>> Sth like below (for the real fix this has to be replicated in
>>> all gimple_resimplifyN functions).  I'm missing a testcase
>>> where the pattern would apply (and not be already folded by fold),
>>> so I didn't check if it actually works.
>> 
>> You do need to check if seq is NULL though as gimple_build depends on
>> seq not being NULL.  But otherwise yes this works for me.
>> 
>>> 
>>> Bah, of course we should fix COND_EXPRs to not embed a GENERIC
>>> expr...
>> 
>> Yes totally agree.  For my changes to tree-ssa-phiopt, I no longer
>> embed it.  Though we need to change loop ifconvert still.
> 
> Istr expansion or code quality does not like us to cse the condition of two cobd_exprs either.  After all I had a patch set at some point doing that conversion (though as well for gimple_conds).

I thought I changed that when I did the expansion of cond_expr into conditional move.  We need to something similar for cond_expr of jumps too. 

Thanks,
Andrew


> 
> Richard.
> 
>> Thanks,
>> Andrew
>> 
>>> 
>>> Richard.
>>> 
>>> Index: gcc/gimple-match-head.c
>>> ===================================================================
>>> --- gcc/gimple-match-head.c     (revision 217035)
>>> +++ gcc/gimple-match-head.c     (working copy)
>>> @@ -90,6 +90,13 @@ gimple_resimplify1 (gimple_seq *seq,
>>>                    code_helper *res_code, tree type, tree *res_ops,
>>>                    tree (*valueize)(tree))
>>> {
>>> +  /* ???  Stupid tcc_comparison GENERIC trees in COND_EXPRs.  */
>>> +  if (COMPARISON_CLASS_P (res_ops[0]))
>>> +    res_ops[0] = gimple_build (seq,
>>> +                              TREE_CODE (res_ops[0]), TREE_TYPE
>> (res_ops[0]),
>>> +                              TREE_OPERAND (res_ops[0], 0),
>>> +                              TREE_OPERAND (res_ops[0], 1));
>>> +
>>>   if (constant_for_folding (res_ops[0]))
>>>     {
>>>       tree tem = NULL_TREE;
>>> 
> 
>