[PATCH] vect, omp: inbranch simdclone dropping const

Andrew Stubbs ams@codesourcery.com
Tue Sep 26 16:37:30 GMT 2023


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


More information about the Gcc-patches mailing list