[PATCH] vect, omp: inbranch simdclone dropping const

[PATCH] vect, omp: inbranch simdclone dropping const

[Andre Vieira (lists)]

[-- Attachment #1: Type: text/plain, Size: 1163 bytes --]

The const attribute is ignored when simdclone's are used inbranch. This 
is due to the fact that when analyzing a MASK_CALL we were not looking 
at the targeted function for flags, but instead only at the internal 
function call itself.
This patch adds code to make sure we look at the target function to 
check for the const attribute and enables the autovectorization of 
inbranch const simdclones without needing the loop to be adorned the 
'openmp simd' pragma.

Not sure about how to add new includes to the ChangeLog. Which brings me 
to another point, I contemplated changing gimple_call_flags to do the 
checking of flags of the first argument of IFN_MASK_CALL itself rather 
than only calling internal_fn_flags on gimple_call_internal_fn (stmt), 
but that might be a bit too intrusive, opinions welcome :)

Bootstrapped and regression tested on aarch64-unknown-linux-gnu and 
x86_64-pc-linux-gnu.

Is this OK for trunk?

gcc/ChangeLog:

         * tree-vect-data-ref.cc (include calls.h): Add new include.
         (get_references_in_stmt): Correctly handle IFN_MASK_CALL.

gcc/testsuite/ChangeLog:

         * gcc.dg/vect/vect-simd-clone-19.c: New test.

[-- Attachment #2: inbranch_simdclone_const.patch --]
[-- Type: text/plain, Size: 2542 bytes --]

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" } */
+
+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..2926c3925ee7897fef53c16cfd1d19d23dbf05f3 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,18 @@ get_references_in_stmt (gimple *stmt, vec<data_ref_loc, va_heap> *references)
 	    }
 	  case IFN_MASK_LOAD:
 	  case IFN_MASK_STORE:
+	  case IFN_MASK_CALL:
+	    {
+	      tree orig_fndecl
+		= gimple_call_addr_fndecl (gimple_call_arg (stmt, 0));
+	      if (!orig_fndecl)
+		{
+		  clobbers_memory = true;
+		  break;
+		}
+	      if ((flags_from_decl_or_type (orig_fndecl) & ECF_CONST) == 0)
+		clobbers_memory = true;
+	    }
 	    break;
 	  default:
 	    clobbers_memory = true;
@@ -5852,7 +5865,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 +5892,15 @@ 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;
 	  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);
 

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

[Andrew Stubbs]

I don't have authority to approve anything, but here's a review anyway.

Thanks for working on this.

On 26/09/2023 17:24, Andre Vieira (lists) wrote:
> The const attribute is ignored when simdclone's are used inbranch. This 
> is due to the fact that when analyzing a MASK_CALL we were not looking 
> at the targeted function for flags, but instead only at the internal 
> function call itself.
> This patch adds code to make sure we look at the target function to 
> check for the const attribute and enables the autovectorization of 
> inbranch const simdclones without needing the loop to be adorned the 
> 'openmp simd' pragma.
> 
> Not sure about how to add new includes to the ChangeLog. Which brings me 
> to another point, I contemplated changing gimple_call_flags to do the 
> checking of flags of the first argument of IFN_MASK_CALL itself rather 
> than only calling internal_fn_flags on gimple_call_internal_fn (stmt), 
> but that might be a bit too intrusive, opinions welcome :)
> 
> Bootstrapped and regression tested on aarch64-unknown-linux-gnu and 
> x86_64-pc-linux-gnu.
> 
> Is this OK for trunk?
> 
> gcc/ChangeLog:
> 
>          * tree-vect-data-ref.cc (include calls.h): Add new include.
>          (get_references_in_stmt): Correctly handle IFN_MASK_CALL.
> 
> gcc/testsuite/ChangeLog:
> 
>          * gcc.dg/vect/vect-simd-clone-19.c: New test.

> 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?

> +	      tree orig_fndecl
> +		= gimple_call_addr_fndecl (gimple_call_arg (stmt, 0));
> +	      if (!orig_fndecl)
> +		{
> +		  clobbers_memory = true;
> +		  break;
> +		}
> +	      if ((flags_from_decl_or_type (orig_fndecl) & ECF_CONST) == 0)
> +		clobbers_memory = true;
> +	    }
>  	    break;
>  

Can be simplified:

   if (!orig_fndecl
       || (flags_from_decl_or_type (orig_fndecl) & ECF_CONST) == 0)
     clobbers_memory = true;
   break;

> @@ -5852,7 +5865,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;
>  

Rogue space.

> @@ -5879,13 +5892,15 @@ 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;
>  	  default:
>  	    break;
>  	  }
>  

If the fall-through is deliberate please add a /* FALLTHROUGH */ comment 
(or whatever spelling disables the warning).

Andrew

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

[Tobias Burnus]

On 26.09.23 18:37, Andrew Stubbs wrote:
> If the fall-through is deliberate please add a /* FALLTHROUGH */
> comment (or whatever spelling disables the warning).

It's: gcc_fallthrough ();

Which gets converted to "__attribute__((fallthrough))"; it could also
expand to "[[fallthrough]]" but that's C++17 (and, also, an C23 feature
- albeit so far unimplemented in gcc).

Tobias

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

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

[Bernhard Reutner-Fischer]

On 26 September 2023 18:46:11 CEST, Tobias Burnus <tobias@codesourcery.com> wrote:
>On 26.09.23 18:37, Andrew Stubbs wrote:
>> If the fall-through is deliberate please add a /* FALLTHROUGH */
>> comment (or whatever spelling disables the warning).
>
>It's: gcc_fallthrough ();
>
>Which gets converted to "__attribute__((fallthrough))"; it could also
>expand to "[[fallthrough]]" but that's C++17 (and, also, an C23 feature
>- albeit so far unimplemented in gcc).

OT
IIRC we do parse comments for a number of spellings of the hint by the user that the fallthrough is deliberate:

https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html

See the numerous levels of -Wimplicit-fallthrough=n, the default being 3.

---8<---
-Wimplicit-fallthrough=3 case sensitively matches one of the following regular expressions:
-fallthrough
@fallthrough@
lint -fallthrough[ \t]*
[ \t.!]*(ELSE,? |INTENTIONAL(LY)? )?
FALL(S | |-)?THR(OUGH|U)[ \t.!]*(-[^\n\r]*)?
[ \t.!]*(Else,? |Intentional(ly)? )?
Fall((s | |-)[Tt]|t)hr(ough|u)[ \t.!]*(-[^\n\r]*)?
[ \t.!]*([Ee]lse,? |[Ii]ntentional(ly)? )?
fall(s | |-)?thr(ough|u)[ \t.!]*(-[^\n\r]*)?
---8<---

Just FWIW.
thanks,

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

[Andre Vieira (lists)]

On 26/09/2023 21:26, Bernhard Reutner-Fischer wrote:
> On 26 September 2023 18:46:11 CEST, Tobias Burnus <tobias@codesourcery.com> wrote:
>> On 26.09.23 18:37, Andrew Stubbs wrote:
>>> If the fall-through is deliberate please add a /* FALLTHROUGH */
>>> comment (or whatever spelling disables the warning).
>>
>> It's: gcc_fallthrough ();
>>
>> Which gets converted to "__attribute__((fallthrough))"; it could also
>> expand to "[[fallthrough]]" but that's C++17 (and, also, an C23 feature
>> - albeit so far unimplemented in gcc).
> 
> OT
> IIRC we do parse comments for a number of spellings of the hint by the user that the fallthrough is deliberate:
> 
> https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
> 
> See the numerous levels of -Wimplicit-fallthrough=n, the default being 3.
> 
> ---8<---
> -Wimplicit-fallthrough=3 case sensitively matches one of the following regular expressions:
> -fallthrough
> @fallthrough@
> lint -fallthrough[ \t]*
> [ \t.!]*(ELSE,? |INTENTIONAL(LY)? )?
> FALL(S | |-)?THR(OUGH|U)[ \t.!]*(-[^\n\r]*)?
> [ \t.!]*(Else,? |Intentional(ly)? )?
> Fall((s | |-)[Tt]|t)hr(ough|u)[ \t.!]*(-[^\n\r]*)?
> [ \t.!]*([Ee]lse,? |[Ii]ntentional(ly)? )?
> fall(s | |-)?thr(ough|u)[ \t.!]*(-[^\n\r]*)?
> ---8<---
> 
> Just FWIW.
> thanks,

I was surprised my bootstrap didn't catch this, I thought we generated 
warnings in such cases and bootstrap builds with -Werror does it not?

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

[Bernhard Reutner-Fischer]

On 26 September 2023 23:02:10 CEST, "Andre Vieira (lists)" <andre.simoesdiasvieira@arm.com> wrote:
>
>
>On 26/09/2023 21:26, Bernhard Reutner-Fischer wrote:
>> On 26 September 2023 18:46:11 CEST, Tobias Burnus <tobias@codesourcery.com> wrote:
>>> On 26.09.23 18:37, Andrew Stubbs wrote:
>>>> If the fall-through is deliberate please add a /* FALLTHROUGH */
>>>> comment (or whatever spelling disables the warning).
>>> 

>
>I was surprised my bootstrap didn't catch this, I thought we generated warnings in such cases and bootstrap builds with -Werror does it not?

Well, I wouldn't see much benefit to warn in this case, no?
You're falling through to a break, not other "active" code AFAICS?

You had:

 	    references->safe_push (ref);
 	    return false;
+	  case IFN_MASK_CALL:
+	    i = 1;
 	  default:
 	    break;
 	  }

I would not have expected a warning here, TBH :-)
thanks,

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

[Andre Vieira (lists)]

[-- Attachment #1: Type: text/plain, Size: 1232 bytes --]



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,
Andre

[-- Attachment #2: inbranch_simdclone_const2.patch --]
[-- Type: text/plain, Size: 2473 bytes --]

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..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);
 

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

[Andrew Stubbs]

On 27/09/2023 08:56, Andre Vieira (lists) wrote:
> 
> 
> 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.

Sorry, I haven't looked closely enough to have an opinion on that, or 
what the side-effects might be.

You have a solution, and like you said, this should be the only case.

I have no further comments on this patch. :)

Andrew

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

[Richard Biener]

On Wed, 27 Sep 2023, Andre Vieira (lists) wrote:

> 
> 
> 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.

The patch is OK.

> 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.

But gimple_call_internal_fn is only half of the work since arguments are
shifted.  So I think handling this in if-conversion and the vectorizer
is the right thing as it's a very short-lived IFN.

Richard.

> Kind Regards,
> Andre
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

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

[Jakub Jelinek]

On Tue, Sep 26, 2023 at 05:24:26PM +0100, Andre Vieira (lists) wrote:
> @@ -5816,6 +5817,18 @@ get_references_in_stmt (gimple *stmt, vec<data_ref_loc, va_heap> *references)
>  	    }
>  	  case IFN_MASK_LOAD:
>  	  case IFN_MASK_STORE:
> +	  case IFN_MASK_CALL:
> +	    {
> +	      tree orig_fndecl
> +		= gimple_call_addr_fndecl (gimple_call_arg (stmt, 0));
> +	      if (!orig_fndecl)
> +		{
> +		  clobbers_memory = true;
> +		  break;
> +		}
> +	      if ((flags_from_decl_or_type (orig_fndecl) & ECF_CONST) == 0)
> +		clobbers_memory = true;
> +	    }

Should IFN_MASK_LOAD/STORE really go through this?  I thought those have
first argument address of the memory being conditionally loaded/stored, not
function address.

	Jakub

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

[Andre Vieira (lists)]

On 26/09/2023 17:48, Jakub Jelinek wrote:
> On Tue, Sep 26, 2023 at 05:24:26PM +0100, Andre Vieira (lists) wrote:
>> @@ -5816,6 +5817,18 @@ get_references_in_stmt (gimple *stmt, vec<data_ref_loc, va_heap> *references)
>>   	    }
>>   	  case IFN_MASK_LOAD:
>>   	  case IFN_MASK_STORE:
>> +	  case IFN_MASK_CALL:
>> +	    {
>> +	      tree orig_fndecl
>> +		= gimple_call_addr_fndecl (gimple_call_arg (stmt, 0));
>> +	      if (!orig_fndecl)
>> +		{
>> +		  clobbers_memory = true;
>> +		  break;
>> +		}
>> +	      if ((flags_from_decl_or_type (orig_fndecl) & ECF_CONST) == 0)
>> +		clobbers_memory = true;
>> +	    }
> 
> Should IFN_MASK_LOAD/STORE really go through this?  I thought those have
> first argument address of the memory being conditionally loaded/stored, not
> function address.
> 
No it shouldn't, my bad...
Surprising testing didn't catch it though, I'm guessing 
gimple_call_addr_fndecl just returned null everytime for those. I'll 
clean it up.