Re: [PATCH] [GCC] AutoFDO: Walk predecessor edges for PHI arguments

Richard Biener <[email protected]> Tue, 4 Aug 2026 09:06:50 +0200
Newsgroups gmane.comp.gcc.patches
Message-ID <CAFiYyc2idXc-09XSuXMGRYz3Y1wZrqdrJR0nJQzDLJSo1Hz9FQ@mail.gmail.com>
On Tue, Aug 4, 2026 at 8:44 AM Jan Hubička <[email protected]> wrote:
>
>
>
> On Tue, Aug 4, 2026 at 2:39 AM Kugan Vivekanandarajah <[email protected]> wrote:
>>
>> PHI arguments are indexed by incoming edges.  function_instance::match
>> walked successor edges and could fail gimple_phi_arg_def_from_edge's
>> destination assertion during afdo_offline.
>>
>> gcc/ChangeLog:
>>
>>         * auto-profile.cc (function_instance::match): Walk predecessor edges
>>         for PHI arguments.
>
>
> This is OK.  It is really strange this does not trigger checking failure
> I see that
> inline location_t
> gimple_phi_arg_location_from_edge (gphi *phi, edge e)
> {
>  return gimple_phi_arg (phi, e->dest_idx)->locus;
>
> Does not check that PHI BB and e->dest match (that looks like omission that would be useful to fix), but still we should eventually hit out of range access which is checked in gimple_phi_arg.

I have put such checking in some of the accessors (because as you guess,
it's "easy" to feed bogus edges into those).  gimple_phi_arg_def_from_edge
it seems.  Similar checking assert would be nice to have in other edge based
PHI accessors.

Richard.

>
> Honza
>>
>>
>> Regression tested on aarch64-linux-gnu with no new regressions
>>
>> Signed-off-by: Kugan Vivekanandarajah <[email protected]>
>> ---
>>  gcc/auto-profile.cc | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/gcc/auto-profile.cc b/gcc/auto-profile.cc
>> index f4562d7f2f98..60f6b1ba480b 100644
>> --- a/gcc/auto-profile.cc
>> +++ b/gcc/auto-profile.cc
>> @@ -1764,7 +1764,8 @@ function_instance::match (cgraph_node *node,
>>           gcc_assert (!info);
>>           dump_stmt (phi, info, NULL, stack);
>>           counts.add (info);
>> -         for (edge e : bb->succs)
>> +         /* PHI arguments are indexed by incoming (predecessor) edges.  */
>> +         for (edge e : bb->preds)
>>             {
>>               location_t phi_loc
>>                 = gimple_phi_arg_location_from_edge (phi, e);
>> --
>> 2.34.1
>>