> This is a repost of:
>
> https://gcc.gnu.org/pipermail/gcc-patches/2020-February/539763.html
>
> which was initially posted during stage 4. (And yeah, I only just
> missed stage 4 again.)
>
> IMO it would be better to fix the bug directly (as the patch tries
> to do) instead of wait for a more thorough redesign of this area.
> See the end of:
>
> https://gcc.gnu.org/pipermail/gcc-patches/2020-February/540002.html
>
> for some stats.
>
> Honza: Richard said he'd like your opinion on the patch.
>
>
> memrefs_conflict_p has a slightly odd structure. It first checks
> whether two addresses based on SYMBOL_REFs refer to the same object,
> with a tristate result:
>
> int cmp = compare_base_symbol_refs (x,y);
>
> If the addresses do refer to the same object, we can use offset-based checks:
>
> /* If both decls are the same, decide by offsets. */
> if (cmp == 1)
> return offset_overlap_p (c, xsize, ysize);
>
> But then, apart from the special case of forced address alignment,
> we use an offset-based check even if we don't know whether the
> addresses refer to the same object:
>
> /* Assume a potential overlap for symbolic addresses that went
> through alignment adjustments (i.e., that have negative
> sizes), because we can't know how far they are from each
> other. */
> if (maybe_lt (xsize, 0) || maybe_lt (ysize, 0))
> return -1;
> /* If decls are different or we know by offsets that there is no
> overlap,
> we win. */
> if (!cmp || !offset_overlap_p (c, xsize, ysize))
> return 0;
>
> This somewhat contradicts:
>
> /* In general we assume that memory locations pointed to by different labels
> may overlap in undefined ways. */
I suppose it is becuase the code above check for SYMBOL_REF and not
label (that is probably about jumptables and constpool injected into
text segment).
I assume this is also bit result of GCC not being very systematic about
aliases. Sometimes it assumes that two different symbols do not point
to same object while in other cases it is worried about aliases.
I see that anchors are special since they point to "same object" with
different offests.
>
> at the end of compare_base_symbol_refs. In other words, we're taking -1
> to mean that either (a) the symbols are equal (via aliasing) or (b) the
> references access non-overlapping objects.
I for symbol refs yes, I think so.
>
> But even assuming that's true for normal symbols, it doesn't cope
> correctly with section anchors. If a symbol X at ANCHOR+OFFSET is
> preemptible, either (a) X = ANCHOR+OFFSET (rather than the X = ANCHOR
> assumed above) or (b) X and ANCHOR reference non-overlapping objects.
>
> And an offset-based comparison makes no sense for an anchor symbol
> vs. a bare symbol with no decl. If the bare symbol is allowed to
> alias other symbols then it can surely alias any symbol in the
> anchor's block, so there are multiple anchor offsets that might
> induce an alias.
>
> This patch therefore replaces the current tristate:
>
> - known equal
> - known independent (two accesses can't alias)
> - equal or independent
>
> with:
>
> - known distance apart
> - known independent (two accesses can't alias)
> - known distance apart or independent
> - don't know
>
> For safety, the patch puts all bare symbols in the "don't know"
> category. If that turns out to be too conservative, we at least
> need that behaviour for combinations involving a bare symbol
> and a section anchor. However, bare symbols should be relatively
> rare these days.
Well, in tree-ssa code we do assume these to be either disjoint objects
or equal (in decl_refs_may_alias_p that continues in case
compare_base_decls is -1). I am not sure if we win much by threating
them differently on RTL level. I would preffer staying consistent here.
Otheriwse the patch looks good to me.
Honza
>
> Retested on aarch64-linux-gnu, aarch64_be-elf and x86_64-linux-gnu.
> OK to install?
>
> Richard
>
>
> gcc/
> PR rtl-optimization/92294
> * alias.c (compare_base_symbol_refs): Take an extra parameter
> and add the distance between two symbols to it. Enshrine in
> comments that -1 means "either 0 or 1, but we can't tell
> which at compile time". Return -2 for symbols whose
> relationship is unknown.
> (memrefs_conflict_p): Update call accordingly.
> (rtx_equal_for_memref_p): Likewise. Punt for a return value of -2,
> without even checking the offset. Take the distance between symbols
> into account.
> ---
> gcc/alias.c | 53 ++++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 38 insertions(+), 15 deletions(-)
>
> diff --git a/gcc/alias.c b/gcc/alias.c
> index 8d3575e4e27..e22863a929a 100644
> --- a/gcc/alias.c
> +++ b/gcc/alias.c
> @@ -159,7 +159,8 @@ static tree decl_for_component_ref (tree);
> static int write_dependence_p (const_rtx,
> const_rtx, machine_mode, rtx,
> bool, bool, bool);
> -static int compare_base_symbol_refs (const_rtx, const_rtx);
> +static int compare_base_symbol_refs (const_rtx, const_rtx,
> + HOST_WIDE_INT * = NULL);
>
> static void memory_modified_1 (rtx, const_rtx, void *);
>
> @@ -1837,7 +1838,11 @@ rtx_equal_for_memref_p (const_rtx x, const_rtx y)
> return label_ref_label (x) == label_ref_label (y);
>
> case SYMBOL_REF:
> - return compare_base_symbol_refs (x, y) == 1;
> + {
> + HOST_WIDE_INT distance = 0;
> + return (compare_base_symbol_refs (x, y, &distance) == 1
> + && distance == 0);
> + }
>
> case ENTRY_VALUE:
> /* This is magic, don't go through canonicalization et al. */
> @@ -2172,10 +2177,24 @@ compare_base_decls (tree base1, tree base2)
> return ret;
> }
>
> -/* Same as compare_base_decls but for SYMBOL_REF. */
> +/* Compare SYMBOL_REFs X_BASE and Y_BASE.
> +
> + - Return 1 if Y_BASE - X_BASE is constant, adding that constant
> + to *DISTANCE if DISTANCE is nonnull.
> +
> + - Return 0 if no valid accesses based on X_BASE can alias valid
> + accesses based on Y_BASE.
> +
> + - Return -1 if one of the two results above applies, but we can't
> + tell which at compile time. Update DISTANCE in the same way as
> + for a return value of 1, for the case in which that result holds
> + at runtime.
> +
> + - Return -2 otherwise. */
>
> static int
> -compare_base_symbol_refs (const_rtx x_base, const_rtx y_base)
> +compare_base_symbol_refs (const_rtx x_base, const_rtx y_base,
> + HOST_WIDE_INT *distance)
> {
> tree x_decl = SYMBOL_REF_DECL (x_base);
> tree y_decl = SYMBOL_REF_DECL (y_base);
> @@ -2195,7 +2214,7 @@ compare_base_symbol_refs (const_rtx x_base, const_rtx
> y_base)
> /* We handle specially only section anchors and assume that other
> labels may overlap with user variables in an arbitrary way. */
> if (!SYMBOL_REF_HAS_BLOCK_INFO_P (y_base))
> - return -1;
> + return -2;
> /* Anchors contains static VAR_DECLs and CONST_DECLs. We are safe
> to ignore CONST_DECLs because they are readonly. */
> if (!VAR_P (x_decl)
> @@ -2222,15 +2241,14 @@ compare_base_symbol_refs (const_rtx x_base, const_rtx
> y_base)
> {
> if (SYMBOL_REF_BLOCK (x_base) != SYMBOL_REF_BLOCK (y_base))
> return 0;
> - if (SYMBOL_REF_BLOCK_OFFSET (x_base) == SYMBOL_REF_BLOCK_OFFSET
> (y_base))
> - return binds_def ? 1 : -1;
> - if (SYMBOL_REF_ANCHOR_P (x_base) != SYMBOL_REF_ANCHOR_P (y_base))
> - return -1;
> - return 0;
> + if (distance)
> + *distance += (SYMBOL_REF_BLOCK_OFFSET (y_base)
> + - SYMBOL_REF_BLOCK_OFFSET (x_base));
> + return binds_def ? 1 : -1;
> }
> /* In general we assume that memory locations pointed to by different
> labels
> may overlap in undefined ways. */
> - return -1;
> + return -2;
> }
>
> /* Return 0 if the addresses X and Y are known to point to different
> @@ -2513,11 +2531,16 @@ memrefs_conflict_p (poly_int64 xsize, rtx x,
> poly_int64 ysize, rtx y,
>
> if (GET_CODE (x) == SYMBOL_REF && GET_CODE (y) == SYMBOL_REF)
> {
> - int cmp = compare_base_symbol_refs (x,y);
> + HOST_WIDE_INT distance = 0;
> + int cmp = compare_base_symbol_refs (x, y, &distance);
>
> - /* If both decls are the same, decide by offsets. */
> + /* Punt if we have no information about the relationship between
> + X and Y. */
> + if (cmp == -2)
> + return -1;
> + /* If the symbols are a known distance apart, decide by offsets. */
> if (cmp == 1)
> - return offset_overlap_p (c, xsize, ysize);
> + return offset_overlap_p (c + distance, xsize, ysize);
> /* Assume a potential overlap for symbolic addresses that went
> through alignment adjustments (i.e., that have negative
> sizes), because we can't know how far they are from each
> @@ -2526,7 +2549,7 @@ memrefs_conflict_p (poly_int64 xsize, rtx x, poly_int64
> ysize, rtx y,
> return -1;
> /* If decls are different or we know by offsets that there is no
> overlap,
> we win. */
> - if (!cmp || !offset_overlap_p (c, xsize, ysize))
> + if (!cmp || !offset_overlap_p (c + distance, xsize, ysize))
> return 0;
> /* Decls may or may not be different and offsets overlap....*/
> return -1;
