Hi Jeff, New version in https://gcc.gnu.org/pipermail/gcc-patches/2026-June/720413.html.
On Thu, Jun 11, 2026 at 6:27 PM Jeffrey Law <[email protected]> wrote: > On 6/11/2026 3:20 AM, Konstantinos Eleftheriou wrote: > > This patch converts the fold-mem-offsets pass from DF to RTL-SSA. > > Along with this conversion, the way the pass collects information > > was completely reworked. Instead of visiting each instruction multiple > > times, this is now done only once. > > > > Most significant changes are: > > * The pass operates mainly on insn_info objects from RTL-SSA. > > * Single iteration over all nondebug INSNs for identification > > of fold-mem-roots. Then walk of the fold-mem-roots' DEF-chain > > to collect foldable constants. > > * The class fold_mem_info holds vectors for the DEF-chain of > > the to-be-folded INSNs (fold_agnostic_insns, which don't need > > to be adjusted, and fold_insns, which need their constant to > > be set to zero). > > * Introduction of a single-USE mode, which only collects DEFs, > > that have a single USE and therefore are safe to transform > > (the fold-mem-root will be the final USE). This mode is fast > > and will always run (unless disabled via -fno-fold-mem-offsets). > > * Introduction of a multi-USE mode, which allows DEFs to have > > multiple USEs, but all USEs must be part of any fold-mem-root's > > DEF-chain. The analysis of all USEs is expensive and therefore, > > this mode is disabled for highly connected CFGs, unless > > -fexpensive-optimizations (enabled by default at -O2) forces it on. > > Note that multi-USE mode will miss some opportunities that the > > single-USE mode finds (e.g. multi-USE mode fails for > > fold-mem-offsets-3.c). > > > > The following testing was done: > > * Bootstrapped and regtested on aarch64-linux, x86-64-linux and > > arm-linux. > > * SPEC CPU tested on aarch64. > > > > A compile time analysis with `/bin/time -v ./install/usr/local/bin/gcc > -O2 all.i` > > (all.i from PR117922) shows: > > * -fno-fold-mem-offsets: 1681 s (user time) / 23626232 kBytes (max > resident set size) > > * -ffold-mem-offsets: 1849 s (user time) / 23625708 kBytes (max > resident set size) > > Multi-USE mode (on by default at -O2 via -fexpensive-optimizations) does > > not have an impact on the duration or the memory footprint. > > > > gcc/ChangeLog: > > > > PR rtl-optimization/117922 > > * fold-mem-offsets.cc (INCLUDE_ALGORITHM): Added definition. > > (INCLUDE_FUNCTIONAL): Likewise. > > (class pass_fold_mem_offsets): Moved to bottom of file. > > (class change_info): New. > > (def_shadowed_by_cond_exec_p): New. > > (get_single_def_in_bb): Converted to RTL-SSA. > > (get_fold_mem_offset_root): Converted to RTL-SSA. > > (get_uses): Removed. > > (fold_offsets): Converted to RTL-SSA. > > (fold_offsets_value): New. > > (fold_offsets_1): Converted to RTL-SSA. > > (has_foldable_uses_p): Converted to RTL-SSA. > > (get_fold_mem_root): Removed. > > (insn_uses_not_in_bitmap): New. > > (drop_unsafe_candidates): New. > > (do_commit_offset): Converted to RTL-SSA. > > (do_analysis): Removed. > > (do_commit_insn): Converted to RTL-SSA. > > (do_fold_info_calculation): Removed. > > (sort_changes): New. > > (struct regno_changes): New. > > (sort_pairs): New. > > (do_check_validity): Removed. > > (get_last_def): New. > > (move_uses_to_prev_def): New. > > (compute_validity_closure): Removed. > > (change_in_vec_p): New. > > (cancel_changes_for_group): New. > > (find_keys_to_remove): New. > > (free_changes_info): New. > > (update_insns): New. > > (fold_mem_offsets_1): New. > > (pass_fold_mem_offsets::execute): Moved to bottom of file. > > (fold_mem_offsets): New. > > > > gcc/testsuite/ChangeLog: > > > > * g++.target/aarch64/fold-mem-offsets.C: New test. > > * gcc.target/aarch64/fold-mem-offsets.c: New test. > > > > Co-authored-by: Christoph Müllner <[email protected]> > Thanks. A lot has changed since I acked an earlier version. I think > the overall state is pretty good and I don't mind iterating if there are > loose ends. > > Just one thing worth pointing out right now; I think we're leaking info > objects in fold_mem_offsets_1: > > > > > -/* If INSN is a move / add instruction that was folded then replace its > > - constant part with zero. */ > > -static void > > -do_commit_insn (rtx_insn *insn) > > +/* Helper function for fold_mem_offsets. Fold memory offsets by > analysing the > > + DEF-USE chain. If SINGLE_USE is true the DEFs will only have a > single use, > > + otherwise they can have multiple uses. */ > > +static unsigned int > > +fold_mem_offsets_1 (bool single_use) > > { > > - if (bitmap_bit_p (&candidate_fold_insns, INSN_UID (insn)) > > - && !bitmap_bit_p (&cannot_fold_insns, INSN_UID (insn))) > > + unsigned int stats_fold_count = 0; > > + > > + /* This maps the instruction changes to the register defined by the > last > > + fold_insn of the def-chain (the one nearest the fold-mem-offset > root). > > + We use this so that we can group interdependent instructions. In > this > > + way, we can restrict the change cancellation in a group only, if > anything > > + goes wrong. */ > > + hash_map<int_hash<unsigned, -1U, -2U>, auto_vec<change_info *>> > changes_map; > > + > > + auto attempt = crtl->ssa->new_change_attempt (); > > + insn_change_watermark watermark; > > + > > + /* Set of removed reg numbers (keys to changes_map). If a change for > a reg > > + number has been cancelled, we need to invalidate any future > changes. */ > > + auto_bitmap removed_regnos; > > + > > + int cancel_min_index = -1; > > + > > + /* Iterate over all nondebug INSNs get our candidates and fold them. > */ > > + auto_vec<fold_mem_info *> worklist; > > + for (auto insn : iterate_safely (crtl->ssa->nondebug_insns ())) > > { > > - if (dump_file) > > + if (!insn->is_real () || !insn->can_be_optimized ()) > > + continue; > > + > > + rtx mem, reg; > > + HOST_WIDE_INT offset; > > + if (!get_fold_mem_offset_root (insn, &mem, ®, &offset)) > > + continue; > > + > > + fold_mem_info *info = new fold_mem_info (insn, mem, reg, offset); > > + > > + if (dump_file && (dump_flags & TDF_DETAILS)) > > { > > - fprintf (dump_file, "Instruction folded:"); > > - print_rtl_single (dump_file, insn); > > + fprintf (dump_file, "Starting analysis from root: "); > > + print_rtl_single (dump_file, info->insn->rtl ()); > > } > > > > - stats_fold_count++; > > + /* Walk DEF-chain and collect info.fold_insns and the resulting > > + offset. */ > > + if (!fold_offsets (info->insn, info->reg, &info->added_offset, > info, > > + single_use) > > + || info->added_offset == 0) > > + continue; > So when we continue the loop here, we leak the most recently allocated > INFO object, right? That seems worth fixing before integration. > Right, fixed it. > > jeff > Thanks, Konstantinos
