On Wed, Dec 17, 2025 at 3:06 AM Alexandre Oliva <[email protected]> wrote: > > On Dec 12, 2025, Alexandre Oliva <[email protected]> wrote: > > > because base terms aren't preserved in cselib, and even with WIP code to > > preserve them, when stack and frame pointer differ by a constant, they > > end up in the same cluster of VALUEs, but alias.cc wants them to have > > different base terms. > > > I suppose one way to improve on this is for the frame pointer after > > reload to share the base term with the stack pointer. This would also > > prevent the sp restore from bp to cause sp's reg_value to be reset, and > > avoid the need for special-casing of the bp set up from sp. Would such > > a change be desirable/acceptable? > > > Is there interest in code to preserve base terms in cselib tables?
IMO "base terms" (aka RTLs "points-to analysis") needs to be re-architected. > I'm posting this WIP patch FTR as reference for > https://gcc.gnu.org/pipermail/gcc-patches/2025-December/703846.html and > to figure out whether there's interest in such an approach to retain > ADDRESSes in cselib tables. This regstraps on x86_64-linux-gnu, but it > would require some further polishing to replace local declarations of > external functions with global ones, drop #if0 and whatnot. > > diff --git a/gcc/alias.cc b/gcc/alias.cc > index a23396eaa35e0..f6f59c8e13119 100644 > --- a/gcc/alias.cc > +++ b/gcc/alias.cc > @@ -1659,6 +1659,16 @@ get_reg_base_value (unsigned int regno) > return (*reg_base_value)[regno]; > } > > +rtx > +get_reg_base_value_checked (unsigned int regno) > +{ > + if (!reg_base_value) > + return NULL_RTX; > + if (regno >= reg_base_value->length ()) > + return NULL_RTX; > + return get_reg_base_value (regno); > +} > + > /* If a value is known for REGNO, return it. */ > > rtx > @@ -1964,6 +1974,12 @@ find_base_term (rtx x, vec<std::pair<cselib_val *, > return temp; > } > > + case VALUE_ADDRESS: > + x = XEXP (x, 0); > + if (GET_CODE (x) != VALUE) > + return x; > + /* Fall through. */ > + > case VALUE: > val = CSELIB_VAL_PTR (x); > ret = NULL_RTX; > @@ -2070,6 +2086,13 @@ find_base_term (rtx x) > return res; > } > > +rtx > +find_value_base_term (rtx x) > +{ > + gcc_checking_assert (GET_CODE (x) == VALUE); > + return find_base_term (x); > +} > + > /* Return true if accesses to address X may alias accesses based > on the stack pointer. */ > > @@ -2315,8 +2338,16 @@ get_addr (rtx x) > for (l = v->locs; l; l = l->next) > if (CONSTANT_P (l->loc)) > return l->loc; > +#if 0 > + else if (GET_CODE (l->loc) == VALUE_ADDRESS) > + return x; > +#endif > for (l = v->locs; l; l = l->next) > if (!REG_P (l->loc) && !MEM_P (l->loc) > + /* ENTRY_VALUEs are not useful addresses, we can't get aliasing > + information from them. */ > + && GET_CODE (l->loc) != ENTRY_VALUE > + && GET_CODE (l->loc) != VALUE_ADDRESS > /* Avoid infinite recursion when potentially dealing with > var-tracking artificial equivalences, by skipping the > equivalences themselves, and not choosing expressions > @@ -2330,6 +2361,8 @@ get_addr (rtx x) > for (l = v->locs; l; l = l->next) > if (REG_P (l->loc) > || (GET_CODE (l->loc) != VALUE > + && GET_CODE (l->loc) != ENTRY_VALUE > + && GET_CODE (l->loc) != VALUE_ADDRESS > && !refs_newer_value_p (l->loc, x))) > return l->loc; > /* Return the canonical value. */ > @@ -3080,6 +3113,38 @@ canon_true_dependence (const_rtx mem, machine_mode > mem_mode, rtx mem_addr, > x, x_addr, /*mem_canonicalized=*/true); > } > > +/* Return true iff x and m are addresses with the same base term, and that > are > + equivalent memrefs. Both are presumed canonicalized. */ > +bool > +same_base_term_known_overlap_p (rtx x, rtx m, poly_int64 size) > +{ > + rtx tx = get_addr (x); > + rtx tm = get_addr (m); > + > + rtx bx = find_base_term (tx); > + rtx bm = find_base_term (tm); > + > + if (bx != bm && bx && bm > + && !(GET_CODE (bx) == ADDRESS > + && GET_CODE (bm) == ADDRESS > + && bx->u.fld[0].rt_int <= 0 > + && bm->u.fld[0].rt_int <= 0) > + /* libgo runtime may different decls for the same symbol used within > the > + same function, because they come from different units compiled > + together. */ > + && !(GET_CODE (bx) == SYMBOL_REF > + && GET_CODE (bm) == SYMBOL_REF > + && XSTR (bx, 0) == XSTR (bm, 0))) > + return false; > + > +#if 0 > + x = canon_rtx (tx); > + m = canon_rtx (tm); > +#endif > + > + return memrefs_conflict_p (size, x, size, m, 0) != 0; > +} > + > /* Returns true if a write to X might alias a previous read from > (or, if WRITEP is true, a write to) MEM. > If X_CANONCALIZED is true, then X_ADDR is the canonicalized address of X, > @@ -3290,16 +3355,25 @@ init_alias_target (void) > && targetm.hard_regno_mode_ok (i, Pmode)) > static_reg_base_value[i] = arg_base_value; > > - /* RTL code is required to be consistent about whether it uses the > - stack pointer, the frame pointer or the argument pointer to > - access a given area of the frame. We can therefore use the > - base address to distinguish between the different areas. */ > + /* RTL code is required to be consistent about whether it uses the stack > + pointer, the frame pointer or the argument pointer to access a given > area > + of the frame. We can therefore use the base address to distinguish > + between the different areas. However, after register allocation and > + especially prologue generation, cselib makes all these areas part of the > + same cluster of VALUEs, making it hard to distinguish the areas, so make > + them all share the same base term then. */ > static_reg_base_value[STACK_POINTER_REGNUM] > - = unique_base_value (UNIQUE_BASE_VALUE_SP); > + = (reload_completed > + ? arg_base_value > + : unique_base_value (UNIQUE_BASE_VALUE_SP)); > static_reg_base_value[ARG_POINTER_REGNUM] > - = unique_base_value (UNIQUE_BASE_VALUE_ARGP); > + = (reload_completed > + ? arg_base_value > + : unique_base_value (UNIQUE_BASE_VALUE_ARGP)); > static_reg_base_value[FRAME_POINTER_REGNUM] > - = unique_base_value (UNIQUE_BASE_VALUE_FP); > + = (reload_completed > + ? arg_base_value > + : unique_base_value (UNIQUE_BASE_VALUE_FP)); > > /* The above rules extend post-reload, with eliminations applying > consistently to each of the three pointers. Cope with cases in > @@ -3307,7 +3381,9 @@ init_alias_target (void) > rather than the stack pointer. */ > if (!HARD_FRAME_POINTER_IS_FRAME_POINTER) > static_reg_base_value[HARD_FRAME_POINTER_REGNUM] > - = unique_base_value (UNIQUE_BASE_VALUE_HFP); > + = (reload_completed > + ? arg_base_value > + : unique_base_value (UNIQUE_BASE_VALUE_HFP)); > } > > /* Set MEMORY_MODIFIED when X modifies DATA (that is assumed > diff --git a/gcc/cselib.cc b/gcc/cselib.cc > index c6628abf2a989..a6cae62bb2fee 100644 > --- a/gcc/cselib.cc > +++ b/gcc/cselib.cc > @@ -2505,6 +2505,8 @@ cselib_lookup (rtx x, machine_mode mode, > static void > cselib_invalidate_regno_val (unsigned int regno, struct elt_list **l) > { > + extern rtx get_reg_base_value_checked (unsigned); > + rtx value_address = get_reg_base_value_checked (regno); > cselib_val *v = (*l)->elt; > if (*l == REG_VALUES (regno)) > { > @@ -2517,7 +2519,12 @@ cselib_invalidate_regno_val (unsigned int regno, > struct elt_list **l) > l = &(*l)->next; > } > else > - unchain_one_elt_list (l); > + { > + if (value_address > + && GET_MODE (value_address) != GET_MODE (v->val_rtx)) > + value_address = NULL_RTX; > + unchain_one_elt_list (l); > + } > > v = canonical_cselib_val (v); > > @@ -2533,6 +2540,56 @@ cselib_invalidate_regno_val (unsigned int regno, > struct elt_list **l) > if (REG_P (x) && REGNO (x) == regno) > { > unchain_one_elt_loc_list (p); > + > + extern rtx find_value_base_term (rtx); > + /* Preserve the VALUE_ADDRESS associated with the regno previously > + held in v, if we can't identify it any longer. */ > + rtx found_value_address = find_value_base_term (v->val_rtx); > + cselib_val *add_address_to = NULL; > + if (value_address && found_value_address != value_address) > + add_address_to = v; > + > + rtx wanted_value_address > + = value_address ? value_address : found_value_address; > + > + if (wanted_value_address) > + { > + cselib_val *addrv = v; > + for (;;) > + { > + rtx addr = canon_rtx (get_addr (addrv->val_rtx)); > + if (addr == addrv->val_rtx > + || GET_CODE (addr) != PLUS > + || GET_CODE (XEXP (addr, 0)) != VALUE > + || !CONSTANT_P (XEXP (addr, 1))) > + break; > + > + rtx addr_value_address > + = find_value_base_term (XEXP (addr, 0)); > + if (addr_value_address) > + break; > + else > + { > + gcc_checking_assert (addr_value_address > + != wanted_value_address); > + addrv = add_address_to > + = canonical_cselib_val (CSELIB_VAL_PTR (XEXP (addr, > 0))); > + } > + } > + } > + > + if (add_address_to) > + { > + if (add_address_to != v) > + p = &add_address_to->locs; > + > + elt_loc_list *n = *p; > + *p = elt_loc_list_pool.allocate (); > + (*p)->loc = gen_rtx_VALUE_ADDRESS (GET_MODE (v->val_rtx), > + wanted_value_address); > + (*p)->setting_insn = setting_insn; > + (*p)->next = n; > + } > break; > } > } > @@ -2628,6 +2685,10 @@ cselib_invalidate_mem (rtx mem_rtx) > if ((v = cselib_lookup (mem_addr, GET_MODE (mem_addr), > 0, GET_MODE (mem_rtx)))) > { > + extern bool same_base_term_known_overlap_p (rtx, rtx, poly_int64); > + gcc_checking_assert (same_base_term_known_overlap_p > + (mem_addr, v->val_rtx, > + GET_MODE_SIZE (GET_MODE (mem_rtx)))); > mem_addr = v->val_rtx; > mem_rtx = replace_equiv_address_nv (mem_rtx, mem_addr); > } > diff --git a/gcc/rtl.def b/gcc/rtl.def > index 15ae7d10fcc10..b1f68fb00d25a 100644 > --- a/gcc/rtl.def > +++ b/gcc/rtl.def > @@ -128,6 +128,9 @@ DEF_RTL_EXPR(SEQUENCE, "sequence", "E", RTX_EXTRA) > /* Represents a non-global base address. This is only used in alias.cc. */ > DEF_RTL_EXPR(ADDRESS, "address", "i", RTX_EXTRA) > > +/* Represents a base address within a cselib table. */ > +DEF_RTL_EXPR(VALUE_ADDRESS, "value_address", "e", RTX_OBJ) > + > /* ---------------------------------------------------------------------- > Expression types used for things in the instruction chain. > > diff --git a/gcc/var-tracking.cc b/gcc/var-tracking.cc > index 8732c3ba62aa5..21cde6b0a7b6c 100644 > --- a/gcc/var-tracking.cc > +++ b/gcc/var-tracking.cc > @@ -2442,6 +2442,7 @@ unsuitable_loc (rtx loc) > case SCRATCH: > case ASM_INPUT: > case ASM_OPERANDS: > + case VALUE_ADDRESS: > return true; > > default: > > > -- > Alexandre Oliva, happy hacker https://blog.lx.oliva.nom.br/ > Free Software Activist FSFLA co-founder GNU Toolchain Engineer > More tolerance and less prejudice are key for inclusion and diversity. > Excluding neuro-others for not behaving ""normal"" is *not* inclusive!
