On Tue, Jan 18, 2022 at 2:40 PM Richard Sandiford via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> In this PR the waccess pass was fed:
>
> D.10779 ={v} {CLOBBER};
> VIEW_CONVERT_EXPR<svuint64_t[2]>(D.10779) = .MASK_LOAD_LANES (addr_5(D),
> 64B, _2);
> _7 = D.10779.__val[0];
>
> However, the tracking of m_clobbers only looked at gassigns,
> so it missed that the clobber on the first line was overwritten
> by the call on the second line.
Just as a note another possible def can come via asm() outputs
and clobbers. There would have been walk_stmt_load_store_ops
to track all those down (not sure if the function is a good fit here).
> This patch splits the updating of m_clobbers out into its own
> function, called after the check_*() routines, and extends it
> to handle both gassigns and gcalls. I think that makes sense
> as an instance of the "read, operate, write" model, with the
> new function being part of "write".
>
> Previously only the gimple_clobber_p handling was conditional
> on m_check_dangling_p, but I think the whole of the new function
> can be. We only enter stmts into m_clobbers if m_check_dangling_p,
> so we only need to remove them under the same condition.
>
> Tested on aarch64-linux-gnu. OK to install?
>
> Richard
>
>
> gcc/
> PR middle-end/104092
> * gimple-ssa-warn-access.cc (pass_waccess::update_clobbers_from_lhs):
> New function, split out from...
> (pass_waccess::check_stmt): ...here and generalized to calls.
> (pass_waccess::check_block): Call it.
>
> gcc/testsuite/
> * gcc.target/aarch64/sve/acle/general/pr104092.c: New test.
> ---
> gcc/gimple-ssa-warn-access.cc | 68 +++++++++++--------
> .../aarch64/sve/acle/general/pr104092.c | 7 ++
> 2 files changed, 48 insertions(+), 27 deletions(-)
> create mode 100644
> gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr104092.c
>
> diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
> index f639807a78a..25066fa6b89 100644
> --- a/gcc/gimple-ssa-warn-access.cc
> +++ b/gcc/gimple-ssa-warn-access.cc
> @@ -2094,6 +2094,9 @@ private:
> /* Check a non-call statement. */
> void check_stmt (gimple *);
>
> + /* Update the clobber map based on the lhs of a statement. */
> + void update_clobbers_from_lhs (gimple *);
> +
> /* Check statements in a basic block. */
> void check_block (basic_block);
>
> @@ -4270,33 +4273,6 @@ is_auto_decl (tree x)
> void
> pass_waccess::check_stmt (gimple *stmt)
> {
> - if (m_check_dangling_p && gimple_clobber_p (stmt))
> - {
> - /* Ignore clobber statemts in blocks with exceptional edges. */
> - basic_block bb = gimple_bb (stmt);
> - edge e = EDGE_PRED (bb, 0);
> - if (e->flags & EDGE_EH)
> - return;
> -
> - tree var = gimple_assign_lhs (stmt);
> - m_clobbers.put (var, stmt);
> - return;
> - }
> -
> - if (is_gimple_assign (stmt))
> - {
> - /* Clobbered unnamed temporaries such as compound literals can be
> - revived. Check for an assignment to one and remove it from
> - M_CLOBBERS. */
> - tree lhs = gimple_assign_lhs (stmt);
> - while (handled_component_p (lhs))
> - lhs = TREE_OPERAND (lhs, 0);
> -
> - if (is_auto_decl (lhs))
> - m_clobbers.remove (lhs);
> - return;
> - }
> -
> if (greturn *ret = dyn_cast <greturn *> (stmt))
> {
> if (optimize && flag_isolate_erroneous_paths_dereference)
> @@ -4326,6 +4302,42 @@ pass_waccess::check_stmt (gimple *stmt)
> }
> }
>
> +/* Update the clobber map based on the lhs of STMT. */
> +
> +void
> +pass_waccess::update_clobbers_from_lhs (gimple *stmt)
> +{
> + if (gimple_clobber_p (stmt))
> + {
> + /* Ignore clobber statements in blocks with exceptional edges. */
> + basic_block bb = gimple_bb (stmt);
> + edge e = EDGE_PRED (bb, 0);
> + if (e->flags & EDGE_EH)
> + return;
> +
> + tree var = gimple_assign_lhs (stmt);
> + m_clobbers.put (var, stmt);
> + return;
> + }
> +
> + if (is_gimple_assign (stmt) || is_gimple_call (stmt))
> + {
> + /* Clobbered unnamed temporaries such as compound literals can be
> + revived. Check for an assignment to one and remove it from
> + M_CLOBBERS. */
> + tree lhs = gimple_get_lhs (stmt);
> + if (!lhs)
> + return;
> +
> + while (handled_component_p (lhs))
> + lhs = TREE_OPERAND (lhs, 0);
> +
> + if (is_auto_decl (lhs))
> + m_clobbers.remove (lhs);
> + return;
> + }
> +}
> +
> /* Check basic block BB for invalid accesses. */
>
> void
> @@ -4340,6 +4352,8 @@ pass_waccess::check_block (basic_block bb)
> check_call (call);
> else
> check_stmt (stmt);
> + if (m_check_dangling_p)
> + update_clobbers_from_lhs (stmt);
> }
> }
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr104092.c
> b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr104092.c
> new file mode 100644
> index 00000000000..c17ece7d82f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr104092.c
> @@ -0,0 +1,7 @@
> +/* { dg-options "-O2 -Wall" } */
> +
> +#include <arm_sve.h>
> +
> +svuint64_t bar(svbool_t pg, const uint64_t *addr) {
> + return svget2(svld2_u64(pg, addr), 0);
> +}
> --
> 2.25.1
>
