On Thu, Apr 17, 2025 at 7:51 PM Andrew Pinski <quic_apin...@quicinc.com> wrote:
>
> The case here is we have:
> ```
> char buf[32] = {};
> void* ret = aaa();
> __builtin_memcpy(ret, buf, 32);
> ```
>
> And buf does not escape. But we don't prop the zeroing from buf to the
> memcpy statement
> because optimize_memcpy_to_memset only looks back one statement. This can be
> fixed to look back
> until we get an statement that may clobber the reference. If we get a phi
> node, then we don't do
> anything.
>
> Bootstrapped and tested on x86_64-linux-gnu.
>
> PR tree-optimization/118947
>
> gcc/ChangeLog:
>
> * gimple-fold.cc (optimize_memcpy_to_memset): Walk back until we get a
> statement that may clobber the read.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/pr118947-1.c: New test.
>
> Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> ---
> gcc/gimple-fold.cc | 39 ++++++++++++++++++++++---------
> gcc/testsuite/gcc.dg/pr118947-1.c | 15 ++++++++++++
> 2 files changed, 43 insertions(+), 11 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/pr118947-1.c
>
> diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
> index 3e6b53d6a0f..94d5a1ebbd7 100644
> --- a/gcc/gimple-fold.cc
> +++ b/gcc/gimple-fold.cc
> @@ -906,18 +906,41 @@ size_must_be_zero_p (tree size)
> static bool
> optimize_memcpy_to_memset (gimple_stmt_iterator *gsip, tree dest, tree src,
> tree len)
> {
> + ao_ref read;
> gimple *stmt = gsi_stmt (*gsip);
> if (gimple_has_volatile_ops (stmt))
> return false;
>
> - tree vuse = gimple_vuse (stmt);
> - if (vuse == NULL || TREE_CODE (vuse) != SSA_NAME)
> - return false;
>
> - gimple *defstmt = SSA_NAME_DEF_STMT (vuse);
> tree src2 = NULL_TREE, len2 = NULL_TREE;
> poly_int64 offset, offset2;
> tree val = integer_zero_node;
> + bool len_was_null = len == NULL_TREE;
> + if (len == NULL_TREE)
> + len = (TREE_CODE (src) == COMPONENT_REF
> + ? DECL_SIZE_UNIT (TREE_OPERAND (src, 1))
> + : TYPE_SIZE_UNIT (TREE_TYPE (src)));
> + if (len == NULL_TREE
> + || !poly_int_tree_p (len))
> + return false;
> +
> + ao_ref_init (&read, src);
> + tree vuse = gimple_vuse (stmt);
> + gimple *defstmt;
> + do {
> + if (vuse == NULL || TREE_CODE (vuse) != SSA_NAME)
> + return false;
> + defstmt = SSA_NAME_DEF_STMT (vuse);
> + if (is_a <gphi*>(defstmt))
> + return false;
> +
> + /* If the len was null, then we can use TBBA. */
> + if (stmt_may_clobber_ref_p_1 (defstmt, &read,
> + /* tbaa_p = */ len_was_null))
> + break;
> + vuse = gimple_vuse (defstmt);
> + } while (true);
I don't think this kind of unbound walk is OK, definitely not from
fold_stmt (). This kind
of walking belongs to a optimization pass.
So, NAK.
Richard.
> +
> if (gimple_store_p (defstmt)
> && gimple_assign_single_p (defstmt)
> && TREE_CODE (gimple_assign_rhs1 (defstmt)) == STRING_CST
> @@ -956,17 +979,11 @@ optimize_memcpy_to_memset (gimple_stmt_iterator *gsip,
> tree dest, tree src, tree
> if (src2 == NULL_TREE)
> return false;
>
> - if (len == NULL_TREE)
> - len = (TREE_CODE (src) == COMPONENT_REF
> - ? DECL_SIZE_UNIT (TREE_OPERAND (src, 1))
> - : TYPE_SIZE_UNIT (TREE_TYPE (src)));
> if (len2 == NULL_TREE)
> len2 = (TREE_CODE (src2) == COMPONENT_REF
> ? DECL_SIZE_UNIT (TREE_OPERAND (src2, 1))
> : TYPE_SIZE_UNIT (TREE_TYPE (src2)));
> - if (len == NULL_TREE
> - || !poly_int_tree_p (len)
> - || len2 == NULL_TREE
> + if (len2 == NULL_TREE
> || !poly_int_tree_p (len2))
> return false;
>
> diff --git a/gcc/testsuite/gcc.dg/pr118947-1.c
> b/gcc/testsuite/gcc.dg/pr118947-1.c
> new file mode 100644
> index 00000000000..70b7f800065
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr118947-1.c
> @@ -0,0 +1,15 @@
> +/* PR tree-optimization/118947 */
> +/* { dg-do compile { target size32plus } } */
> +/* { dg-options "-O2 -fdump-tree-forwprop1-details" } */
> +/* { dg-final { scan-tree-dump-times "after previous" 1 "forwprop1" } } */
> +
> +void* aaa();
> +void* bbb()
> +{
> + char buf[32] = {};
> + /* Tha call to aaa should not matter and clobber buf. */
> + void* ret = aaa();
> + __builtin_memcpy(ret, buf, 32);
> + return ret;
> +}
> +
> --
> 2.43.0
>
