Re: [PATCH] vect, omp: inbranch simdclone dropping const

[Andre Vieira (lists)]

On 26/09/2023 17:37, Andrew Stubbs wrote:
I don't have authority to approve anything, but here's a review anyway.

Thanks for working on this.

Thank you for reviewing and apologies for the mess of a patch, may have 
rushed it ;)

diff --git a/gcc/testsuite/gcc.dg/vect/vect-simd-clone-19.c 
b/gcc/testsuite/gcc.dg/vect/vect-simd-clone-19.c
new file mode 100644
index 
0000000000000000000000000000000000000000..09127b8cb6f2e3699b6073591f58be7047330273
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/vect-simd-clone-19.c
@@ -0,0 +1,23 @@
+/* { dg-require-effective-target vect_simd_clones } */
+/* { dg-do compile } */
+/* { dg-additional-options "-fopenmp-simd" } */
+

Do you need -fopenmp-simd for this?
Nope, I keep forgetting that you only need it for pragmas.

Dealt with the other comments too.

Any thoughts on changing gimple_call_internal_fn  instead? My main 
argument against is that IFN_MASK_CALL should not appear outside of 
ifconvert and vectorizer. On the other hand, we may inspect the flags 
elsewhere in the vectorizer (now or in the future) and changing 
gimple_call_internal_fn would prevent the need to handle the IFN 
separately elsewhere.

Kind Regards,
Andrediff --git a/gcc/testsuite/gcc.dg/vect/vect-simd-clone-19.c 
b/gcc/testsuite/gcc.dg/vect/vect-simd-clone-19.c
new file mode 100644
index 
0000000000000000000000000000000000000000..e7ed56ca75470464307d0d266dacfa0d8d6e43c1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/vect-simd-clone-19.c
@@ -0,0 +1,22 @@
+/* { dg-require-effective-target vect_simd_clones } */
+/* { dg-do compile } */
+
+int __attribute__ ((__simd__, const)) fn (int);
+
+void test (int * __restrict__ a, int * __restrict__ b, int n)
+{
+  for (int i = 0; i < n; ++i)
+    {
+      int a_;
+      if (b[i] > 0)
+        a_ = fn (b[i]);
+      else
+        a_ = b[i] + 5;
+      a[i] = a_;
+    }
+}
+
+/* { dg-final { scan-tree-dump-not {loop contains function calls or data 
references} "vect" } } */
+
+/* The LTO test produces two dump files and we scan the wrong one.  */
+/* { dg-skip-if "" { *-*-* } { "-flto" } { "" } } */
diff --git a/gcc/tree-data-ref.cc b/gcc/tree-data-ref.cc
index 
6d3b7c2290e4db9c1168a4c763facb481157c97c..689aaeed72282bb0da2a17e19fb923a06e8d62fa
 100644
--- a/gcc/tree-data-ref.cc
+++ b/gcc/tree-data-ref.cc
@@ -100,6 +100,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "vr-values.h"
 #include "range-op.h"
 #include "tree-ssa-loop-ivopts.h"
+#include "calls.h"
 
 static struct datadep_stats
 {
@@ -5816,6 +5817,15 @@ get_references_in_stmt (gimple *stmt, vec<data_ref_loc, 
va_heap> *references)
            }
          case IFN_MASK_LOAD:
          case IFN_MASK_STORE:
+         break;
+         case IFN_MASK_CALL:
+           {
+             tree orig_fndecl
+               = gimple_call_addr_fndecl (gimple_call_arg (stmt, 0));
+             if (!orig_fndecl
+                 || (flags_from_decl_or_type (orig_fndecl) & ECF_CONST) == 0)
+               clobbers_memory = true;
+           }
            break;
          default:
            clobbers_memory = true;
@@ -5852,7 +5862,7 @@ get_references_in_stmt (gimple *stmt, vec<data_ref_loc, 
va_heap> *references)
     }
   else if (stmt_code == GIMPLE_CALL)
     {
-      unsigned i, n;
+      unsigned i = 0, n;
       tree ptr, type;
       unsigned int align;
 
@@ -5879,13 +5889,16 @@ get_references_in_stmt (gimple *stmt, vec<data_ref_loc, 
va_heap> *references)
                                   ptr);
            references->safe_push (ref);
            return false;
+         case IFN_MASK_CALL:
+           i = 1;
+           gcc_fallthrough ();
          default:
            break;
          }
 
       op0 = gimple_call_lhs (stmt);
       n = gimple_call_num_args (stmt);
-      for (i = 0; i < n; i++)
+      for (; i < n; i++)
        {
          op1 = gimple_call_arg (stmt, i);