Hi Honza.
> On 2 Dec 2025, at 8:23 pm, Jan Hubicka <[email protected]> wrote: > > External email: Use caution opening links or attachments > > >> gcc/ChangeLog: >> >> * cfgloopmanip.cc (duplicate_loop_body_to_header_edge): Assign >> hierarchical discriminators for loop unrolling. >> * cfgloopmanip.h (DLTHE_RECORD_HIERARCHICAL_DISCRIMINATOR): New flag. >> * tree-ssa-loop-ivcanon.cc (try_unroll_loop_completely): Pass flag >> to enable hierarchical discriminator assignment. >> (try_peel_loop): Likewise. >> >> gcc/testsuite/ChangeLog: >> >> * gcc.dg/hierarchical-discriminator-unroll.c: New test. >> >> Signed-off-by: Kugan Vivekanandarajah <[email protected]> >> --- >> gcc/cfgloopmanip.cc | 51 +++++++++++++++++++ >> gcc/cfgloopmanip.h | 4 ++ >> .../hierarchical-discriminator-unroll.c | 33 ++++++++++++ >> gcc/tree-ssa-loop-ivcanon.cc | 7 ++- >> 4 files changed, 93 insertions(+), 2 deletions(-) >> create mode 100644 gcc/testsuite/gcc.dg/hierarchical-discriminator-unroll.c >> >> diff --git a/gcc/cfgloopmanip.cc b/gcc/cfgloopmanip.cc >> index dda2fb661d8..7cf5aea5a0f 100644 >> --- a/gcc/cfgloopmanip.cc >> +++ b/gcc/cfgloopmanip.cc >> @@ -34,6 +34,7 @@ along with GCC; see the file COPYING3. If not see >> #include "sreal.h" >> #include "tree-cfg.h" >> #include "tree-pass.h" >> +#include "hierarchical_discriminator.h" >> >> static void copy_loops_to (class loop **, int, >> class loop *); >> @@ -1422,6 +1423,56 @@ duplicate_loop_body_to_header_edge (class loop *loop, >> edge e, >> new_bbs[i]->aux = (void *)(size_t)(j + 1); >> } >> >> + /* Assign hierarchical discriminators to distinguish loop iterations. >> */ >> + if (flags & DLTHE_RECORD_HIERARCHICAL_DISCRIMINATOR) >> + { >> + /* Only handle GIMPLE mode for now. */ >> + if (current_ir_type () == IR_GIMPLE) >> + { >> + /* For loop unrolling: >> + - multiplicity = unroll factor (ndupl for complete unrolling) >> + - copyid = keep original (don't change) >> + All unrolled iterations share the same copyid but >> + have multiplicity set. */ >> + unsigned int unroll_factor = ndupl; >> + >> + for (i = 0; i < n; i++) >> + { >> + for (gimple_stmt_iterator gsi = gsi_start_bb (new_bbs[i]); >> + !gsi_end_p (gsi); gsi_next (&gsi)) >> + { >> + gimple *stmt = gsi_stmt (gsi); >> + location_t loc = gimple_location (stmt); >> + >> + if (loc == UNKNOWN_LOCATION || !is_gimple_debug (stmt)) >> + continue; >> + unsigned int base, old_multiplicity, old_copyid; >> + get_discriminator_components_from_loc (loc, &base, >> + &old_multiplicity, >> + &old_copyid); > > I am bit confused here. My reading of what multiplicity means is that > given location correspond to multiplicity executions of a given > statement. This happens with vectorization (including SLP one) where we > turn multiple scalar statements to one or when the statement was > optimized out. > > With unrolling we produce multiple copies of a given statement which I > think shoudl correspond to different copy_ids not different > multiplicities? I misunderstood the previous review. I have now updated with copy_id in the attached patch. > > Also your patchset is missing loop header copying and jump threading > which is likely quite a common case of duplication. I will post a seperate patch for this. Thanks, Kugan > > Honza
0004-Autofdo-V2-Add-hierarchical-discriminator-for-loop-u.patch
Description: 0004-Autofdo-V2-Add-hierarchical-discriminator-for-loop-u.patch
