Re: [PATCH] waccess: Look at calls when tracking clobbers [PR104092]

[Martin Sebor via Gcc-patches]

On 1/18/22 06:37, Richard Sandiford 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.

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.

Thanks for the patch.  If you or someone can think of a test case
that's independent of a target, adding one would be very helpful.

Other than that, since I can't approve any changes I CC Jason who
was kind enough to approve the implementation of the warning for
his OK.

Martin

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);
+}