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 >>