It seems to me that extending operand_equal_p to deal with the MEM_REF/ARRAY_REF case could help.
Your recent change of:
if (TREE_CODE (arg0) == MEM_REF
&& DECL_P (arg1)
&& TREE_CODE (TREE_OPERAND (arg0, 0)) == ADDR_EXPR
&& TREE_OPERAND (TREE_OPERAND (arg0, 0), 0) == arg1
&& integer_zerop (TREE_OPERAND (arg0, 1)))
return 1;
else if (TREE_CODE (arg1) == MEM_REF
&& DECL_P (arg0)
&& TREE_CODE (TREE_OPERAND (arg1, 0)) == ADDR_EXPR
&& TREE_OPERAND (TREE_OPERAND (arg1, 0), 0) == arg0
&& integer_zerop (TREE_OPERAND (arg1, 1)))
return 1;
return 0;
only seems to be handing the case of 'X' and MEM_REF[&X + 0]. Do you think
changing this to be 'return addr_eq_p (arg0, arg1);' where addr_eq_p handles
cases of ARRAY_(RANGED_)REF, COMPONENT_REF by checking offset as well
would be useful?
I've put together an initial version which splits out change to operand_equal_p
into its own predicate for just IDing cases of base objects and the above
mentioned addr_eq_p. Also in that patch is the change for mem_attrs_eq_p as
in that case the offset is not part of the TREE expression, so it has to be
handled
differently.
Thoughts?
Thanks,
Simon
-----Original Message-----
From: Richard Biener [mailto:[email protected]]
Sent: 22 September 2015 12:12
To: Simon Dardis
Cc: Jeff Law; [email protected]
Subject: Re: Predictive commoning leads to register to register moves through
memory.
On Tue, Sep 22, 2015 at 12:45 PM, Simon Dardis <[email protected]> wrote:
> I took an attempt at addressing this through the RTL GCSE pass. This
> attempt tweaks mem_attrs_eq_p to return true if its comparing something like
> poly+8 and MEM [&poly + 8].
>
> Is this a more suitable approach?
I actually recently modified stmt_kills_ref_p for a similar reason...
not that I liked that vey much.
I think the more suitable approach would be to not have both forms if they are
actually equal.
Of course that's way more work.
So splitting out a function that handles sematic equality compare of MEM_EXPR
sounds good to me.
No comments on your actual implementation yet, but I think we should do it in a
way to be re-usable by stmt_kills_ref_p.
Richard.
> Thanks,
> Simon
>
> +/* Return true if p and q reference the same location by the same name but
> + through VAR_DECL and MEM_REF. */
> +
> +static bool
> +mem_locations_match_p (const struct mem_attrs *p, const struct
> +mem_attrs *q) {
> + HOST_WIDE_INT var_offset;
> + tree var, memref;
> +
> + if (p->expr == NULL_TREE || q->expr == NULL_TREE)
> + return false;
> +
> + if (TREE_CODE (p->expr) == MEM_REF && TREE_CODE (q->expr) == VAR_DECL)
> + {
> + var = q->expr;
> + var_offset = q->offset;
> + memref = p->expr;
> + }
> + else if (TREE_CODE (q->expr) == MEM_REF && TREE_CODE (p->expr) == VAR_DECL)
> + {
> + var = p->expr;
> + var_offset = p->offset;
> + memref = q->expr;
> + }
> + else
> + return false;
> +
> + if (TREE_OPERAND (TREE_OPERAND (memref, 0), 0) != var)
> + return false;
> +
> + if (TREE_TYPE (TREE_TYPE (var)) != TREE_TYPE (memref))
> + return false;
> +
> + tree offset = TREE_OPERAND (memref, 1); if ((TREE_CODE (offset) ==
> + INTEGER_CST && tree_fits_shwi_p (offset)
> + && tree_to_shwi (offset) == var_offset)
> + || offset == NULL_TREE && var_offset == 0)
> + return true;
> +
> + return false;
> +
> +}
> +
> /* Return true if the given memory attributes are equal. */
>
> bool
> @@ -254,16 +298,16 @@ mem_attrs_eq_p (const struct mem_attrs *p, const struct
> mem_attrs *q)
> return false;
> return (p->alias == q->alias
> && p->offset_known_p == q->offset_known_p
> - && (!p->offset_known_p || p->offset == q->offset)
> && p->size_known_p == q->size_known_p
> && (!p->size_known_p || p->size == q->size)
> && p->align == q->align
> && p->addrspace == q->addrspace
> - && (p->expr == q->expr
> - || (p->expr != NULL_TREE && q->expr != NULL_TREE
> - && operand_equal_p (p->expr, q->expr, 0))));
> + && (mem_locations_match_p (p, q)
> + || (!p->offset_known_p || p->offset == q->offset)
> + && (p->expr == q->expr
> + || (p->expr != NULL_TREE && q->expr != NULL_TREE
> + && operand_equal_p (p->expr, q->expr, 0)))));
> }
gcse.patch
Description: gcse.patch
