It has been tested on AArch64, x86-64 and ARM. I forgot to add the info. Thanks, Konstantinos
On Sat, Sep 19, 2026 at 8:11 PM Jeff Law <[email protected]> wrote: > > > On 9/4/26 4:51 AM, Konstantinos Eleftheriou wrote: > > Once a fold insn's constant is folded into the memory offsets computed > from > > it, the pass zeroes that constant. This is only correct if every root > reached > > through the insn takes the compensating offset, so roots whose def-chains > > share a fold insn must be committed, or cancelled, as a unit. > > > > The changes were grouped by the register defined by the last fold insn > of the > > def-chain, which misses a base add shared between chains that end in > different > > registers. In the testcase, g_13, g_590 and g_253 are all reached > through one > > section-anchor add, r0 = r7 + 976. The g_590 loads reach it via a second > add > > and are grouped under r3, so committing them zeroes the shared add. The > g_253 > > load is grouped under r0 and needs offset 1008, outside LDRSB's range, > so its > > group is cancelled, but the zeroing survives: the g_13 and g_253 > accesses are > > left at r7 + {4, 32} instead of r7 + {980, 1008}. > > > > Group the changes by the connected components of the roots that share a > fold > > insn, maintained as a union-find, so that a root that cannot be updated > rolls > > back its whole component. Also commit all surviving groups in one > change_insns > > call, which removes the sort over the group keys. > > > > PR rtl-optimization/126412 > > > > gcc/ChangeLog: > > > > * fold-mem-offsets.cc (change_in_vec_p): Move before fold_groups. > > (changes_map_t): New type. > > (class fold_groups): New class. > > (struct regno_changes): Remove. > > (sort_pairs): Likewise. > > (cancel_changes_for_group): Take the fold group to cancel. > > (find_groups_to_cancel): Rename from find_keys_to_remove. > > (update_insns): Group the changes by fold group instead of by the > > register defined by the last fold insn of the def-chain. > > (fold_mem_offsets_1): Use fold_groups and commit all groups in a > > single change_insns call. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.target/arm/pr126412.c: New test. > > --- > > > I don't see any information on testing. OK assuming it's been > bootstrapped and regression tested on one of the usual primary targets. > > jeff >
