Re: [PATCH] Improve memrefs_conflict_p with VALUEs

[Richard Guenther <richard dot guenther at gmail dot com>]

On Fri, Apr 16, 2010 at 9:18 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> When cselib processes typical i386 fn argument pushing like:
> (insn:TI 5 23 6 2 a.c:5 (set (mem:SI (plus:SI (reg/f:SI 7 sp)
> ? ? ? ? ? ? ? ?(const_int 8 [0x8])) [0 S4 A32])
> ? ? ? ?(const_int 3 [0x3])) 47 {*movsi_1} (nil))
> (insn:TI 6 5 7 2 a.c:5 (set (mem:SI (plus:SI (reg/f:SI 7 sp)
> ? ? ? ? ? ? ? ?(const_int 4 [0x4])) [0 S4 A32])
> ? ? ? ?(const_int 2 [0x2])) 47 {*movsi_1} (nil))
> (insn:TI 7 6 8 2 a.c:5 (set (mem:SI (reg/f:SI 7 sp) [0 S4 A32])
> ? ? ? ?(const_int 1 [0x1])) 47 {*movsi_1} (nil))
> the second MEM store invalidates the first one and the third one
> invalidates the second one. ?The problem is when cselib_invalidate_mem
> is called, it calls canon_true_dependence where mem_addr is
> not using any VALUEs - (plus:SI (reg/f:SI 7 sp) (const_int 4 [0x4]))
> or (reg/f:SI 7 sp), but the other MEM (x) has address substed to values.
> find_base_term sees they are using the same base term (sp) as it iterates
> over VALUE locations, but memrefs_conflict_p doesn't (it unconditionally
> calls get_addr which ignores REGs and MEMs in ->locs list and returns
> something else if anything, or the VALUE itself), so memrefs_conflict_p
> doesn't find out that the MEMs are certainly non-overlapping in this
> case.
>
> The following patch improves it for VALUE == VALUE pair (if the value is
> the same, it would unnecessarily get_addr on both sides and possibly expand
> to something that memrefs_conflict_p can't handle) and when one address
> is a VALUE and another is a REG - in that case it checks if VALUE isn't the
> current VALUE of the register.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2010-04-16 ?Jakub Jelinek ?<jakub@redhat.com>
>
> ? ? ? ?* alias.c (memrefs_conflict_p): If x and y are the same VALUE,
> ? ? ? ?don't call get_addr on both. ?If one expression is a VALUE and
> ? ? ? ?the other a REG, check VALUE's locs if the REG isn't among them.
>
> --- gcc/alias.c.jj ? ? ?2010-04-07 16:19:53.000000000 +0200
> +++ gcc/alias.c 2010-04-15 20:00:34.000000000 +0200
> @@ -1789,9 +1789,39 @@ static int
> ?memrefs_conflict_p (int xsize, rtx x, int ysize, rtx y, HOST_WIDE_INT c)
> ?{
> ? if (GET_CODE (x) == VALUE)
> - ? ?x = get_addr (x);
> + ? ?{
> + ? ? ?if (REG_P (y))
> + ? ? ? {
> + ? ? ? ? struct elt_loc_list *l;
> + ? ? ? ? for (l = CSELIB_VAL_PTR (x)->locs; l; l = l->next)
> + ? ? ? ? ? if (REG_P (l->loc) && rtx_equal_for_memref_p (l->loc, y))
> + ? ? ? ? ? ? break;
> + ? ? ? ? if (l)
> + ? ? ? ? ? x = y;
> + ? ? ? ? else
> + ? ? ? ? ? x = get_addr (x);
> + ? ? ? }
> + ? ? ?/* Don't call get_addr if y is the same VALUE. ?*/
> + ? ? ?else if (x != y)
> + ? ? ? x = get_addr (x);
> + ? ?}
> ? if (GET_CODE (y) == VALUE)
> - ? ?y = get_addr (y);
> + ? ?{
> + ? ? ?if (REG_P (x))
> + ? ? ? {
> + ? ? ? ? struct elt_loc_list *l;
> + ? ? ? ? for (l = CSELIB_VAL_PTR (y)->locs; l; l = l->next)
> + ? ? ? ? ? if (REG_P (l->loc) && rtx_equal_for_memref_p (l->loc, x))
> + ? ? ? ? ? ? break;
> + ? ? ? ? if (l)
> + ? ? ? ? ? y = x;
> + ? ? ? ? else
> + ? ? ? ? ? y = get_addr (y);
> + ? ? ? }
> + ? ? ?/* Don't call get_addr if x is the same VALUE. ?*/
> + ? ? ?else if (y != x)
> + ? ? ? y = get_addr (y);
> + ? ?}
> ? if (GET_CODE (x) == HIGH)
> ? ? x = XEXP (x, 0);
> ? else if (GET_CODE (x) == LO_SUM)
>
> ? ? ? ?Jakub
>