On Tue, Feb 4, 2020 at 6:44 PM Richard Sandiford
<richard.sandif...@arm.com> wrote:
>
> Richard Sandiford <richard.sandif...@arm.com> writes:
> > [...]
> >> I'm not sure given the issues you've introduced if I could actually
> >> fill out the matrix of answers without more underlying information.
> >> ie, when can we get symbols without source level decls,
> >> anchors+interposition issues, etc.
> >
> > OK. In that case, I wonder whether it would be safer to have a
> > fourth state on top of the three above:
> >
> > - known distance apart
> > - independent
> > - known distance apart or independent
> > - don't know
> >
> > with "don't know" being anything that involves bare symbols?
> >
> > Richard
>
> How does this look? Tested on aarch64-linux-gnu and
> x86_64-linux-gnu.
>
> Full description from scratch:
>
> 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. */
Sorry for not chiming in earlier but isn't the bug that
> 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.
>
> 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 = ANDHOR+OFFSET 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.
But then isn't the simple fix to honor the -1 and do
diff --git a/gcc/alias.c b/gcc/alias.c
index 3794f9b6a9e..bf13d37c0f7 100644
--- a/gcc/alias.c
+++ b/gcc/alias.c
@@ -2490,9 +2490,8 @@ memrefs_conflict_p (poly_int64 xsize, rtx x,
poly_int64 ysize, rtx y,
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))
+ /* If decls are different, we win. */
+ if (cmp == 0)
return 0;
/* Decls may or may not be different and offsets overlap....*/
return -1;
?
> 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.
This sounds complicated (for this stage). Do you have any statistics as
to how it affects actual alias queries (thus outcome in {true,...}_dependence)
when you do the "simple" fix?
Thanks,
Richard.
> 2020-02-04 Richard Sandiford <richard.sandif...@arm.com>
>
> 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 3794f9b6a9e..c8b53df0b48 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 *);
>
> @@ -1806,7 +1807,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. */
> @@ -2138,10 +2143,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);
> @@ -2161,7 +2180,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)
> @@ -2188,15 +2207,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
> @@ -2479,11 +2497,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
> @@ -2492,7 +2515,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;
