Re: [PATCH] Handle PHI nodes w/o a argument (PR ipa/80205).

[Richard Biener <richard.guenther@gmail.com>]

On Tue, Mar 28, 2017 at 9:45 AM, Martin Liška <mliska@suse.cz> wrote:
> On 03/27/2017 04:27 PM, Richard Biener wrote:
>> On Mon, Mar 27, 2017 at 4:14 PM, Richard Biener
>> <richard.guenther@gmail.com> wrote:
>>> On Mon, Mar 27, 2017 at 2:47 PM, Martin Liška <mliska@suse.cz> wrote:
>>>> Hello.
>>>>
>>>> As described in the PR, we can create a PHI node in einline that has no argument.
>>>> That can cause ICE in devirtualization and should be thus handled.
>>>>
>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>>>
>>>> Ready to be installed?
>>>
>>> We shouldn't ever have a PHI w/o argument.
>>
>> ;;   basic block 14, loop depth 0
>> ;;    pred:
>>   # SR.2_19 = PHI <>
>> <L4> [0.00%]:
>>   a::~a (&g);
>>   resx 12
>> ;;    succ:       17
>>
>> the CFG has not been cleaned up here (this block is unreachable).
>>
>> Hmm, I see we are called from fold_stmt.  I suppose we process
>> statements_to_fold before removing unreachable blocks to not
>> walk over dead stmts...  chicken-and-egg...
>>
>> Still not creating that PHI should be possible.
>
> I see, thanks for help. Patch fixes the issue, may I install it after
> regression tested?

I think it's progression, still not 100% safe as we have a SSA name with
a GIMPLE_NOP definition that is not in any BB and the SSA name is not
a default def.

But yes, I've written a comment to that effect ;)

Thus ok.

An improvement would be to not copy unreachable regions at all...

Thanks,
Richard.

> Thanks,
> Martin
>
>>
>> Index: gcc/tree-inline.c
>> ===================================================================
>> --- gcc/tree-inline.c   (revision 246500)
>> +++ gcc/tree-inline.c   (working copy)
>> @@ -2344,6 +2344,13 @@ copy_phis_for_bb (basic_block bb, copy_b
>>        if (!virtual_operand_p (res))
>>         {
>>           walk_tree (&new_res, copy_tree_body_r, id, NULL);
>> +         if (EDGE_COUNT (new_bb->preds) == 0)
>> +           {
>> +             /* Technically we'd want a SSA_DEFAULT_DEF here... */
>> +             SSA_NAME_DEF_STMT (new_res) = gimple_build_nop ();
>> +           }
>> +         else
>> +           {
>>           new_phi = create_phi_node (new_res, new_bb);
>>           FOR_EACH_EDGE (new_edge, ei, new_bb->preds)
>>             {
>> @@ -2389,6 +2396,7 @@ copy_phis_for_bb (basic_block bb, copy_b
>>
>>               add_phi_arg (new_phi, new_arg, new_edge, locus);
>>             }
>> +           }
>>         }
>>      }
>>
>>
>>
>>>> Martin
>