On 9/11/19 10:22 AM, Li Jia He wrote:
> Hi,
> 
> On 2019/9/10 3:40 PM, Martin Liška wrote:
>> On 9/9/19 3:55 PM, Richard Biener wrote:
>>> On Mon, 9 Sep 2019, Martin Liška wrote:
>>>
>>>> On 9/9/19 3:42 PM, Richard Biener wrote:
>>>>> There is no newly created GIMPLE?
>>>>
>>>> Hm, I thought from the beginning that maybe_fold_comparisons_from_match_pd
>>>> can come up with new temporary expressions that need to be inserted into
>>>> GIMPLE stream? But that's probably handled in ifcombine with:
>>>>
>>>>       t = force_gimple_operand_gsi_1 (&gsi, t, is_gimple_condexpr, NULL, 
>>>> true,
>>>>                       GSI_SAME_STMT);
>>>> ?
>>>
>>> No, that case is done when forcing short-circuiting when there was no
>>> simplification.  When there was a simplification we do
>>>
>>>        if (result_inv)
>>>          t = fold_build1 (TRUTH_NOT_EXPR, TREE_TYPE (t), t);
>>>        t = canonicalize_cond_expr_cond (t);
>>>        if (!t)
>>>          return false;
>>>
>>> so when it is not a condition suitable for direct replacement into
>>>
>>>        gimple_cond_set_condition_from_tree (inner_cond, t);
>>>
>>> we fail.
>>>
>>> Richard.
>>>
>>
>> I see, so I'm sending updated tested patch.
> 

Hello.

Next time, please Reply to all ;)

> Thanks for working on this.
> I tested the patch.  But I found that this patch will cause gcc not to 
> bootstrap.  I found the following statement may be a question.
> '
> gimple *stmt1 = (gimple *) XALLOCAVEC (char, gimple_size (GIMPLE_ASSIGN, 2));
> '
> we may need to change it to the following
> '
> gimple *stmt1 = (gimple *) XALLOCAVEC (char, gimple_size (GIMPLE_ASSIGN, 3));
> '
> And we may also replace gimple with gassign.

Good point, I'll include the changes to my patchset.

> 
> And we may also need to make some changes to the statements in
> if(op.resimplify (NULL, follow_all_ssa_edges)).  Please take a look at the 
> attachment:

@@ -5888,23 +5890,22 @@

 maybe_fold_comparisons_from_match_pd (tree type, enum tree_code code,

       if (gimple_simplified_result_is_gimple_val (&op)) 
        { 
          tree res = op.ops[0]; 
-         switch (TREE_CODE (res)) 
-           { 
-           case SSA_NAME: 
-               { 
-                 if (res == lhs1) 
-                   return build2 (code1, type, op1a, op1b); 
-                 else if (res == lhs2) 
-                   return build2 (code2, type, op2a, op2b); 
-                 else 
-                   return res; 
-               } 
-           case INTEGER_CST: 
-             /* Fold expression to boolean_true_node or boolean_false_node.  
*/ 
-             return res; 
-           default: 
-             return NULL_TREE; 
-           } 
+         if (res == lhs1) 
+           return build2 (code1, type, op1a, op1b); 
+         else if (res == lhs2) 
+           return build2 (code2, type, op2a, op2b); 
+         else 
+           return res; 

I'll include this.

+       } 
+ 
+      if (op.code.is_tree_code () 
+         && TREE_CODE_CLASS ((enum tree_code) op.code) == tcc_comparison) 
+       { 
+         tree op0 = op.ops[0]; 
+         tree op1 = op.ops[1]; 
+         if (op0 == lhs1 || op0 == lhs2 || op1 == lhs1 || op1 == lhs2) 
+           return NULL_TREE; /* not simple */ 
+         return build2 ((enum tree_code) op.code, op.type, op0, op1); 
        } 
     } 

This is added in patch 3/5.

Martin

> 
> Thanks,
> Lijia He
> 
>>
>> Martin

Reply via email to