__builtin_prefetch fixes [PR117608]

  Hi!

The r15-4833-ge9ab41b79933 patch had among tons of config/i386
specific changes also important change to the generic code, allowing
also 2 as valid value of the second argument of __builtin_prefetch:
-  /* Argument 1 must be either zero or one.  */                                                                                                                                      
-  if (INTVAL (op1) != 0 && INTVAL (op1) != 1)                                                                                                                                        
+  /* Argument 1 must be 0, 1 or 2.  */                                                                                                                                               
+  if (INTVAL (op1) < 0 || INTVAL (op1) > 2)                                                                                                                                          

But the patch failed to document that change in __builtin_prefetch
documentation, and more importantly didn't adjust any of the other
backends to deal with it (my understanding is the expected behavior
is that 2 will be silently handled as 0 unless backends have some
more specific way).  Some of the backends would ICE on it, in some
cases gcc_assert failures/gcc_unreachable, in other cases crash later
(e.g. accessing arrays with that value as index and due to accessing
garbage after the array crashing at final.cc time), others treated 2
silently as 0, others treated 2 silently as 1.

And even in the i386 backend there were bugs which caused ICEs.
The patch added some if (write == 0) and write 2 handling into
a (badly indented, maybe that is the reason, if (write == 1) body),
rather than into the else side, so it would be always false.

The new *prefetch_rst2 define_insn only accepts parameters 2 1
(i.e. read-shared with moderate degree of locality), so in order
not to ICE the patch uses it only for __builtin_prefetch (ptr, 2, 1);
or __builtin_ia32_prefetch (ptr, 2, 1, 0); and not for other values
of the parameter.  If that isn't what we want and we want it to be used
also for all or some of __builtin_prefetch (ptr, 2, {0,2,3}); and
corresponding __builtin_ia32_prefetch, maybe the define_insn could match
other values.
And there was another problem that -mno-mmx -mno-sse -mmovrs compilation
would ICE on most of the prefetches, so I had to add the FAIL; cases.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2024-11-27  Jakub Jelinek  <jakub@redhat.com>

	PR target/117608
	* doc/extend.texi (__builtin_prefetch): Document that second
	argument may be also 2 and its meaning.
	* config/i386/i386.md (prefetch): Remove unreachable code.
	Clear write set operands[1] to const0_rtx if !TARGET_MOVRS or
	of locality is not 1.  Formatting fixes.
	* config/i386/i386-expand.cc (ix86_expand_builtin): Use IN_RANGE.
	Call gen_prefetch even for TARGET_MOVRS.
	* config/alpha/alpha.md (prefetch): Treat read_or_write 2 like 0.
	* config/mips/mips.md (prefetch): Likewise.
	* config/arc/arc.md (prefetch_1, prefetch_2, prefetch_3): Likewise.
	* config/riscv/riscv.md (prefetch): Likewise.
	* config/loongarch/loongarch.md (prefetch): Likewise.
	* config/sparc/sparc.md (prefetch): Likewise.  Use IN_RANGE.
	* config/ia64/ia64.md (prefetch): Likewise.
	* config/pa/pa.md (prefetch): Likewise.
	* config/aarch64/aarch64.md (prefetch): Likewise.
	* config/rs6000/rs6000.md (prefetch): Likewise.

	* gcc.dg/builtin-prefetch-1.c (good): Add tests with second argument
	2.
	* gcc.target/i386/pr117608-1.c: New test.
	* gcc.target/i386/pr117608-2.c: New test.


	Jakub
  

On Wed, 27 Nov 2024, Jakub Jelinek wrote:

> Hi!
> 
> The r15-4833-ge9ab41b79933 patch had among tons of config/i386
> specific changes also important change to the generic code, allowing
> also 2 as valid value of the second argument of __builtin_prefetch:
> -  /* Argument 1 must be either zero or one.  */                                                                                                                                      
> -  if (INTVAL (op1) != 0 && INTVAL (op1) != 1)                                                                                                                                        
> +  /* Argument 1 must be 0, 1 or 2.  */                                                                                                                                               
> +  if (INTVAL (op1) < 0 || INTVAL (op1) > 2)                                                                                                                                          
> 
> But the patch failed to document that change in __builtin_prefetch
> documentation, and more importantly didn't adjust any of the other
> backends to deal with it (my understanding is the expected behavior
> is that 2 will be silently handled as 0 unless backends have some
> more specific way).  Some of the backends would ICE on it, in some
> cases gcc_assert failures/gcc_unreachable, in other cases crash later
> (e.g. accessing arrays with that value as index and due to accessing
> garbage after the array crashing at final.cc time), others treated 2
> silently as 0, others treated 2 silently as 1.
> 
> And even in the i386 backend there were bugs which caused ICEs.
> The patch added some if (write == 0) and write 2 handling into
> a (badly indented, maybe that is the reason, if (write == 1) body),
> rather than into the else side, so it would be always false.
> 
> The new *prefetch_rst2 define_insn only accepts parameters 2 1
> (i.e. read-shared with moderate degree of locality), so in order
> not to ICE the patch uses it only for __builtin_prefetch (ptr, 2, 1);
> or __builtin_ia32_prefetch (ptr, 2, 1, 0); and not for other values
> of the parameter.  If that isn't what we want and we want it to be used
> also for all or some of __builtin_prefetch (ptr, 2, {0,2,3}); and
> corresponding __builtin_ia32_prefetch, maybe the define_insn could match
> other values.
> And there was another problem that -mno-mmx -mno-sse -mmovrs compilation
> would ICE on most of the prefetches, so I had to add the FAIL; cases.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK, please leave other (target maintainers) time to comment.

Richard.

> 2024-11-27  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR target/117608
> 	* doc/extend.texi (__builtin_prefetch): Document that second
> 	argument may be also 2 and its meaning.
> 	* config/i386/i386.md (prefetch): Remove unreachable code.
> 	Clear write set operands[1] to const0_rtx if !TARGET_MOVRS or
> 	of locality is not 1.  Formatting fixes.
> 	* config/i386/i386-expand.cc (ix86_expand_builtin): Use IN_RANGE.
> 	Call gen_prefetch even for TARGET_MOVRS.
> 	* config/alpha/alpha.md (prefetch): Treat read_or_write 2 like 0.
> 	* config/mips/mips.md (prefetch): Likewise.
> 	* config/arc/arc.md (prefetch_1, prefetch_2, prefetch_3): Likewise.
> 	* config/riscv/riscv.md (prefetch): Likewise.
> 	* config/loongarch/loongarch.md (prefetch): Likewise.
> 	* config/sparc/sparc.md (prefetch): Likewise.  Use IN_RANGE.
> 	* config/ia64/ia64.md (prefetch): Likewise.
> 	* config/pa/pa.md (prefetch): Likewise.
> 	* config/aarch64/aarch64.md (prefetch): Likewise.
> 	* config/rs6000/rs6000.md (prefetch): Likewise.
> 
> 	* gcc.dg/builtin-prefetch-1.c (good): Add tests with second argument
> 	2.
> 	* gcc.target/i386/pr117608-1.c: New test.
> 	* gcc.target/i386/pr117608-2.c: New test.
> 
> --- gcc/doc/extend.texi.jj	2024-11-26 09:37:56.173574966 +0100
> +++ gcc/doc/extend.texi	2024-11-26 17:49:35.469152396 +0100
> @@ -15675,9 +15675,11 @@ be in the cache by the time it is access
>  
>  The value of @var{addr} is the address of the memory to prefetch.
>  There are two optional arguments, @var{rw} and @var{locality}.
> -The value of @var{rw} is a compile-time constant one or zero; one
> -means that the prefetch is preparing for a write to the memory address
> -and zero, the default, means that the prefetch is preparing for a read.
> +The value of @var{rw} is a compile-time constant zero, one or two; one
> +means that the prefetch is preparing for a write to the memory address,
> +two means that the prefetch is preparing for a shared read (expected to be
> +read by at least one other processor before it is written if written at
> +all) and zero, the default, means that the prefetch is preparing for a read.
>  The value @var{locality} must be a compile-time constant integer between
>  zero and three.  A value of zero means that the data has no temporal
>  locality, so it need not be left in the cache after the access.  A value
> --- gcc/config/i386/i386.md.jj	2024-11-26 09:37:56.047576735 +0100
> +++ gcc/config/i386/i386.md	2024-11-26 18:18:34.662828702 +0100
> @@ -28654,8 +28654,7 @@
>    [(prefetch (match_operand 0 "address_operand")
>  	     (match_operand:SI 1 "const_int_operand")
>  	     (match_operand:SI 2 "const_int_operand"))]
> -  "TARGET_3DNOW || TARGET_PREFETCH_SSE || TARGET_PRFCHW
> -  || TARGET_MOVRS"
> +  "TARGET_3DNOW || TARGET_PREFETCH_SSE || TARGET_PRFCHW || TARGET_MOVRS"
>  {
>    int write = INTVAL (operands[1]);
>    int locality = INTVAL (operands[2]);
> @@ -28668,7 +28667,7 @@
>       SSE prefetch is not available (K6 machines).  Otherwise use SSE
>       prefetch as it allows specifying of locality.  */
>  
> -      if (write == 1)
> +  if (write == 1)
>      {
>        if (TARGET_PRFCHW)
>  	operands[2] = GEN_INT (3);
> @@ -28676,33 +28675,29 @@
>  	operands[2] = GEN_INT (3);
>        else if (TARGET_PREFETCH_SSE)
>  	operands[1] = const0_rtx;
> -      else if (write == 0)
> -	{
> -	  gcc_assert (TARGET_3DNOW);
> -	  operands[2] = GEN_INT (3);
> -	}
> +      else if (TARGET_3DNOW)
> +	operands[2] = GEN_INT (3);
>        else
>  	{
> -	  if (TARGET_MOVRS)
> -	    ;
> -	  else if (TARGET_PREFETCH_SSE)
> -	    operands[1] = const0_rtx;
> -	  else
> -	    {
> -	      gcc_assert (TARGET_3DNOW);
> -	      operands[1] = const0_rtx;
> -              operands[2] = GEN_INT (3);
> -	    }
> +	  gcc_assert (TARGET_MOVRS);
> +	  FAIL;
>  	}
>      }
>    else
>      {
> -      if (TARGET_PREFETCH_SSE)
> +      if (!TARGET_MOVRS || locality != 1)
> +	{
> +	  operands[1] = const0_rtx;
> +	  write = 0;
> +	}
> +      if (TARGET_PREFETCH_SSE || write == 2)
>  	;
> +      else if (TARGET_3DNOW)
> +	operands[2] = GEN_INT (3);
>        else
>  	{
> -	  gcc_assert (TARGET_3DNOW);
> -	  operands[2] = GEN_INT (3);
> +	  gcc_assert (TARGET_MOVRS);
> +	  FAIL;
>  	}
>      }
>  })
> --- gcc/config/i386/i386-expand.cc.jj	2024-11-26 09:37:56.019577128 +0100
> +++ gcc/config/i386/i386-expand.cc	2024-11-26 18:03:04.095841152 +0100
> @@ -14222,7 +14222,7 @@ ix86_expand_builtin (tree exp, rtx targe
>  
>  	if (INTVAL (op3) == 1)
>  	  {
> -	    if (INTVAL (op2) < 2 || INTVAL (op2) > 3)
> +	    if (!IN_RANGE (INTVAL (op2), 2, 3))
>  	      {
>  		error ("invalid third argument");
>  		return const0_rtx;
> @@ -14252,14 +14252,16 @@ ix86_expand_builtin (tree exp, rtx targe
>  		op0 = copy_addr_to_reg (op0);
>  	      }
>  
> -	    if (INTVAL (op2) < 0 || INTVAL (op2) > 3)
> +	    if (!IN_RANGE (INTVAL (op2), 0, 3))
>  	      {
>  		warning (0, "invalid third argument to %<__builtin_ia32_prefetch%>; using zero");
>  		op2 = const0_rtx;
>  	      }
>  
> -	    if (TARGET_3DNOW || TARGET_PREFETCH_SSE
> -		|| TARGET_PRFCHW)
> +	    if (TARGET_3DNOW
> +		|| TARGET_PREFETCH_SSE
> +		|| TARGET_PRFCHW
> +		|| TARGET_MOVRS)
>  	      emit_insn (gen_prefetch (op0, op1, op2));
>  	    else if (!MEM_P (op0) && side_effects_p (op0))
>  	      /* Don't do anything with direct references to volatile memory,
> --- gcc/config/alpha/alpha.md.jj	2024-11-18 09:05:00.210283154 +0100
> +++ gcc/config/alpha/alpha.md	2024-11-26 17:35:34.620915135 +0100
> @@ -5171,7 +5171,7 @@ (define_insn "prefetch"
>      }
>    };
>  
> -  bool write = INTVAL (operands[1]) != 0;
> +  bool write = (INTVAL (operands[1]) & 1) != 0;
>    bool lru = INTVAL (operands[2]) != 0;
>  
>    return alt[write][lru];
> --- gcc/config/mips/mips.md.jj	2024-09-03 16:48:01.609846676 +0200
> +++ gcc/config/mips/mips.md	2024-11-26 17:40:24.016866697 +0100
> @@ -7395,6 +7395,8 @@ (define_insn "prefetch"
>        else
>  	return "lw\t$0,%a0";
>      }
> +  if (operands[1] == const2_rtx)
> +    operands[1] = const0_rtx;
>    /* Loongson ext2 implementation pref instructions.  */
>    if (TARGET_LOONGSON_EXT2)
>      {
> --- gcc/config/arc/arc.md.jj	2024-01-03 11:51:48.947489474 +0100
> +++ gcc/config/arc/arc.md	2024-11-26 17:37:17.131481085 +0100
> @@ -5439,9 +5439,9 @@ (define_insn "prefetch_1"
>  	     (match_operand:SI 2 "const_int_operand" "n"))]
>    "TARGET_HS"
>    {
> -   if (INTVAL (operands[1]))
> +    if ((INTVAL (operands[1]) & 1) != 0)
>        return "prefetchw [%0]";
> -   else
> +    else
>        return "prefetch [%0]";
>    }
>    [(set_attr "type" "load")
> @@ -5454,9 +5454,9 @@ (define_insn "prefetch_2"
>  	     (match_operand:SI 3 "const_int_operand" "n,n,n"))]
>    "TARGET_HS"
>    {
> -   if (INTVAL (operands[2]))
> +    if ((INTVAL (operands[2]) & 1) != 0)
>        return "prefetchw\\t[%0, %1]";
> -   else
> +    else
>        return "prefetch\\t[%0, %1]";
>    }
>    [(set_attr "type" "load")
> @@ -5468,10 +5468,10 @@ (define_insn "prefetch_3"
>  	     (match_operand:SI 2 "const_int_operand" "n"))]
>    "TARGET_HS"
>    {
> -   operands[0] = gen_rtx_MEM (SImode, operands[0]);
> -   if (INTVAL (operands[1]))
> +    operands[0] = gen_rtx_MEM (SImode, operands[0]);
> +    if ((INTVAL (operands[1]) & 1) != 0)
>        return "prefetchw%U0\\t%0";
> -   else
> +    else
>        return "prefetch%U0\\t%0";
>     }
>    [(set_attr "type" "load")
> --- gcc/config/riscv/riscv.md.jj	2024-11-20 09:29:17.965421565 +0100
> +++ gcc/config/riscv/riscv.md	2024-11-26 17:41:49.684668268 +0100
> @@ -4219,7 +4219,8 @@ (define_insn "prefetch"
>  {
>    switch (INTVAL (operands[1]))
>    {
> -    case 0: return TARGET_ZIHINTNTL ? "%L2prefetch.r\t%a0" : "prefetch.r\t%a0";
> +    case 0:
> +    case 2: return TARGET_ZIHINTNTL ? "%L2prefetch.r\t%a0" : "prefetch.r\t%a0";
>      case 1: return TARGET_ZIHINTNTL ? "%L2prefetch.w\t%a0" : "prefetch.w\t%a0";
>      default: gcc_unreachable ();
>    }
> --- gcc/config/loongarch/loongarch.md.jj	2024-10-08 09:54:47.395484057 +0200
> +++ gcc/config/loongarch/loongarch.md	2024-11-26 17:38:56.063097105 +0100
> @@ -4091,7 +4091,8 @@ (define_insn "prefetch"
>  {
>    switch (INTVAL (operands[1]))
>    {
> -    case 0: return "preld\t0,%a0";
> +    case 0:
> +    case 2: return "preld\t0,%a0";
>      case 1: return "preld\t8,%a0";
>      default: gcc_unreachable ();
>    }
> --- gcc/config/sparc/sparc.md.jj	2024-11-25 09:32:32.494140049 +0100
> +++ gcc/config/sparc/sparc.md	2024-11-26 17:44:24.705499648 +0100
> @@ -7832,9 +7832,9 @@ (define_insn "prefetch_64"
>    int read_or_write = INTVAL (operands[1]);
>    int locality = INTVAL (operands[2]);
>  
> -  gcc_assert (read_or_write == 0 || read_or_write == 1);
> -  gcc_assert (locality >= 0 && locality < 4);
> -  return prefetch_instr [read_or_write][locality == 0 ? 0 : 1];
> +  gcc_assert (IN_RANGE (read_or_write, 0, 2));
> +  gcc_assert (IN_RANGE (locality, 0, 3));
> +  return prefetch_instr [read_or_write & 1][locality == 0 ? 0 : 1];
>  }
>    [(set_attr "type" "load")
>     (set_attr "subtype" "prefetch")])
> @@ -7858,9 +7858,9 @@ (define_insn "prefetch_32"
>    int read_or_write = INTVAL (operands[1]);
>    int locality = INTVAL (operands[2]);
>  
> -  gcc_assert (read_or_write == 0 || read_or_write == 1);
> -  gcc_assert (locality >= 0 && locality < 4);
> -  return prefetch_instr [read_or_write][locality == 0 ? 0 : 1];
> +  gcc_assert (IN_RANGE (read_or_write, 0, 2));
> +  gcc_assert (IN_RANGE (locality, 0, 3));
> +  return prefetch_instr [read_or_write & 1][locality == 0 ? 0 : 1];
>  }
>    [(set_attr "type" "load")
>     (set_attr "subtype" "prefetch")])
> --- gcc/config/ia64/ia64.md.jj	2024-10-11 11:30:38.647392761 +0200
> +++ gcc/config/ia64/ia64.md	2024-11-26 17:38:13.556691733 +0100
> @@ -5041,9 +5041,9 @@ (define_insn "prefetch"
>    int i = (INTVAL (operands[1]));
>    int j = (INTVAL (operands[2]));
>  
> -  gcc_assert (i == 0 || i == 1);
> -  gcc_assert (j >= 0 && j <= 3);
> -  return alt[i][j];
> +  gcc_assert (IN_RANGE (i, 0, 2));
> +  gcc_assert (IN_RANGE (j, 0, 3));
> +  return alt[i & 1][j];
>  }
>    [(set_attr "itanium_class" "lfetch")])
>  
> --- gcc/config/pa/pa.md.jj	2024-11-26 09:37:56.074576356 +0100
> +++ gcc/config/pa/pa.md	2024-11-26 17:41:11.558201627 +0100
> @@ -10354,7 +10354,7 @@ (define_insn "prefetch_20"
>        "ldd 0(%0),%%r0"
>      }
>    };
> -  int read_or_write = INTVAL (operands[1]) == 0 ? 0 : 1;
> +  int read_or_write = INTVAL (operands[1]) & 1;
>    int locality = INTVAL (operands[2]) == 0 ? 0 : 1;
>  
>    return instr [locality][read_or_write];
> --- gcc/config/aarch64/aarch64.md.jj	2024-11-21 09:32:35.264366968 +0100
> +++ gcc/config/aarch64/aarch64.md	2024-11-26 17:34:29.648824047 +0100
> @@ -1020,7 +1020,7 @@ (define_insn "prefetch"
>         the address into a DImode MEM so that aarch64_print_operand knows
>         how to print it.  */
>      operands[0] = gen_rtx_MEM (DImode, operands[0]);
> -    return pftype[INTVAL(operands[1])][locality];
> +    return pftype[INTVAL (operands[1]) & 1][locality];
>    }
>    [(set_attr "type" "load_4")]
>  )
> --- gcc/config/rs6000/rs6000.md.jj	2024-11-21 09:32:35.411364879 +0100
> +++ gcc/config/rs6000/rs6000.md	2024-11-26 17:42:30.845092465 +0100
> @@ -14353,14 +14353,14 @@ (define_insn "prefetch"
>  
>    if (REG_P (operands[0]))
>      {
> -      if (INTVAL (operands[1]) == 0)
> +      if (INTVAL (operands[1]) != 1)
>          return inst_select ? "dcbt 0,%0" : "dcbt 0,%0,16";
>        else
>          return inst_select ? "dcbtst 0,%0" : "dcbtst 0,%0,16";
>      }
>    else
>      {
> -      if (INTVAL (operands[1]) == 0)
> +      if (INTVAL (operands[1]) != 1)
>          return inst_select ? "dcbt %a0" : "dcbt %a0,16";
>        else
>          return inst_select ? "dcbtst %a0" : "dcbtst %a0,16";
> --- gcc/testsuite/gcc.dg/builtin-prefetch-1.c.jj	2024-11-01 19:32:19.177964137 +0100
> +++ gcc/testsuite/gcc.dg/builtin-prefetch-1.c	2024-11-26 17:56:00.493766587 +0100
> @@ -23,6 +23,10 @@ good (int *p)
>    __builtin_prefetch (p, 1, 1);
>    __builtin_prefetch (p, 1, 2);
>    __builtin_prefetch (p, 1, 3);
> +  __builtin_prefetch (p, 2, 0);
> +  __builtin_prefetch (p, 2, 1);
> +  __builtin_prefetch (p, 2, 2);
> +  __builtin_prefetch (p, 2, 3);
>  }
>  
>  void
> --- gcc/testsuite/gcc.target/i386/pr117608-1.c.jj	2024-11-26 18:03:34.775411999 +0100
> +++ gcc/testsuite/gcc.target/i386/pr117608-1.c	2024-11-26 18:22:08.573837662 +0100
> @@ -0,0 +1,14 @@
> +/* PR target/117608 */
> +/* { dg-do compile } */
> +/* { dg-options "-mno-movrs" } */
> +
> +int i;
> +
> +void
> +foo (void)
> +{
> +  __builtin_ia32_prefetch (&i, 2, 0, 0);
> +  __builtin_ia32_prefetch (&i, 2, 1, 0);
> +  __builtin_ia32_prefetch (&i, 2, 2, 0);
> +  __builtin_ia32_prefetch (&i, 2, 3, 0);
> +}
> --- gcc/testsuite/gcc.target/i386/pr117608-2.c.jj	2024-11-26 18:20:47.385972882 +0100
> +++ gcc/testsuite/gcc.target/i386/pr117608-2.c	2024-11-26 18:21:27.995405054 +0100
> @@ -0,0 +1,14 @@
> +/* PR target/117608 */
> +/* { dg-do compile } */
> +/* { dg-options "-mno-mmx -mno-sse -mmovrs" } */
> +
> +int i;
> +
> +void
> +foo (void)
> +{
> +  __builtin_ia32_prefetch (&i, 2, 0, 0);
> +  __builtin_ia32_prefetch (&i, 2, 1, 0);
> +  __builtin_ia32_prefetch (&i, 2, 2, 0);
> +  __builtin_ia32_prefetch (&i, 2, 3, 0);
> +}
> 
> 	Jakub
> 
>
  

On Wed, Nov 27, 2024 at 8:50 PM Richard Biener <rguenther@suse.de> wrote:
>
> On Wed, 27 Nov 2024, Jakub Jelinek wrote:
>
> > Hi!
> >
> > The r15-4833-ge9ab41b79933 patch had among tons of config/i386
> > specific changes also important change to the generic code, allowing
> > also 2 as valid value of the second argument of __builtin_prefetch:
> > -  /* Argument 1 must be either zero or one.  */
> > -  if (INTVAL (op1) != 0 && INTVAL (op1) != 1)
> > +  /* Argument 1 must be 0, 1 or 2.  */
> > +  if (INTVAL (op1) < 0 || INTVAL (op1) > 2)
> >
> > But the patch failed to document that change in __builtin_prefetch
> > documentation, and more importantly didn't adjust any of the other
> > backends to deal with it (my understanding is the expected behavior
> > is that 2 will be silently handled as 0 unless backends have some
> > more specific way).  Some of the backends would ICE on it, in some
> > cases gcc_assert failures/gcc_unreachable, in other cases crash later
> > (e.g. accessing arrays with that value as index and due to accessing
> > garbage after the array crashing at final.cc time), others treated 2
> > silently as 0, others treated 2 silently as 1.
> >
> > And even in the i386 backend there were bugs which caused ICEs.
> > The patch added some if (write == 0) and write 2 handling into
> > a (badly indented, maybe that is the reason, if (write == 1) body),
> > rather than into the else side, so it would be always false.
> >
> > The new *prefetch_rst2 define_insn only accepts parameters 2 1
> > (i.e. read-shared with moderate degree of locality), so in order
> > not to ICE the patch uses it only for __builtin_prefetch (ptr, 2, 1);
> > or __builtin_ia32_prefetch (ptr, 2, 1, 0); and not for other values
> > of the parameter.  If that isn't what we want and we want it to be used
> > also for all or some of __builtin_prefetch (ptr, 2, {0,2,3}); and
> > corresponding __builtin_ia32_prefetch, maybe the define_insn could match
> > other values.
> > And there was another problem that -mno-mmx -mno-sse -mmovrs compilation
> > would ICE on most of the prefetches, so I had to add the FAIL; cases.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> OK, please leave other (target maintainers) time to comment.
LGTM for x86 part, thanks for handling this.
>
> Richard.
>
> > 2024-11-27  Jakub Jelinek  <jakub@redhat.com>
> >
> >       PR target/117608
> >       * doc/extend.texi (__builtin_prefetch): Document that second
> >       argument may be also 2 and its meaning.
> >       * config/i386/i386.md (prefetch): Remove unreachable code.
> >       Clear write set operands[1] to const0_rtx if !TARGET_MOVRS or
> >       of locality is not 1.  Formatting fixes.
> >       * config/i386/i386-expand.cc (ix86_expand_builtin): Use IN_RANGE.
> >       Call gen_prefetch even for TARGET_MOVRS.
> >       * config/alpha/alpha.md (prefetch): Treat read_or_write 2 like 0.
> >       * config/mips/mips.md (prefetch): Likewise.
> >       * config/arc/arc.md (prefetch_1, prefetch_2, prefetch_3): Likewise.
> >       * config/riscv/riscv.md (prefetch): Likewise.
> >       * config/loongarch/loongarch.md (prefetch): Likewise.
> >       * config/sparc/sparc.md (prefetch): Likewise.  Use IN_RANGE.
> >       * config/ia64/ia64.md (prefetch): Likewise.
> >       * config/pa/pa.md (prefetch): Likewise.
> >       * config/aarch64/aarch64.md (prefetch): Likewise.
> >       * config/rs6000/rs6000.md (prefetch): Likewise.
> >
> >       * gcc.dg/builtin-prefetch-1.c (good): Add tests with second argument
> >       2.
> >       * gcc.target/i386/pr117608-1.c: New test.
> >       * gcc.target/i386/pr117608-2.c: New test.
> >
> > --- gcc/doc/extend.texi.jj    2024-11-26 09:37:56.173574966 +0100
> > +++ gcc/doc/extend.texi       2024-11-26 17:49:35.469152396 +0100
> > @@ -15675,9 +15675,11 @@ be in the cache by the time it is access
> >
> >  The value of @var{addr} is the address of the memory to prefetch.
> >  There are two optional arguments, @var{rw} and @var{locality}.
> > -The value of @var{rw} is a compile-time constant one or zero; one
> > -means that the prefetch is preparing for a write to the memory address
> > -and zero, the default, means that the prefetch is preparing for a read.
> > +The value of @var{rw} is a compile-time constant zero, one or two; one
> > +means that the prefetch is preparing for a write to the memory address,
> > +two means that the prefetch is preparing for a shared read (expected to be
> > +read by at least one other processor before it is written if written at
> > +all) and zero, the default, means that the prefetch is preparing for a read.
> >  The value @var{locality} must be a compile-time constant integer between
> >  zero and three.  A value of zero means that the data has no temporal
> >  locality, so it need not be left in the cache after the access.  A value
> > --- gcc/config/i386/i386.md.jj        2024-11-26 09:37:56.047576735 +0100
> > +++ gcc/config/i386/i386.md   2024-11-26 18:18:34.662828702 +0100
> > @@ -28654,8 +28654,7 @@
> >    [(prefetch (match_operand 0 "address_operand")
> >            (match_operand:SI 1 "const_int_operand")
> >            (match_operand:SI 2 "const_int_operand"))]
> > -  "TARGET_3DNOW || TARGET_PREFETCH_SSE || TARGET_PRFCHW
> > -  || TARGET_MOVRS"
> > +  "TARGET_3DNOW || TARGET_PREFETCH_SSE || TARGET_PRFCHW || TARGET_MOVRS"
> >  {
> >    int write = INTVAL (operands[1]);
> >    int locality = INTVAL (operands[2]);
> > @@ -28668,7 +28667,7 @@
> >       SSE prefetch is not available (K6 machines).  Otherwise use SSE
> >       prefetch as it allows specifying of locality.  */
> >
> > -      if (write == 1)
> > +  if (write == 1)
> >      {
> >        if (TARGET_PRFCHW)
> >       operands[2] = GEN_INT (3);
> > @@ -28676,33 +28675,29 @@
> >       operands[2] = GEN_INT (3);
> >        else if (TARGET_PREFETCH_SSE)
> >       operands[1] = const0_rtx;
> > -      else if (write == 0)
> > -     {
> > -       gcc_assert (TARGET_3DNOW);
> > -       operands[2] = GEN_INT (3);
> > -     }
> > +      else if (TARGET_3DNOW)
> > +     operands[2] = GEN_INT (3);
> >        else
> >       {
> > -       if (TARGET_MOVRS)
> > -         ;
> > -       else if (TARGET_PREFETCH_SSE)
> > -         operands[1] = const0_rtx;
> > -       else
> > -         {
> > -           gcc_assert (TARGET_3DNOW);
> > -           operands[1] = const0_rtx;
> > -              operands[2] = GEN_INT (3);
> > -         }
> > +       gcc_assert (TARGET_MOVRS);
> > +       FAIL;
> >       }
> >      }
> >    else
> >      {
> > -      if (TARGET_PREFETCH_SSE)
> > +      if (!TARGET_MOVRS || locality != 1)
> > +     {
> > +       operands[1] = const0_rtx;
> > +       write = 0;
> > +     }
> > +      if (TARGET_PREFETCH_SSE || write == 2)
> >       ;
> > +      else if (TARGET_3DNOW)
> > +     operands[2] = GEN_INT (3);
> >        else
> >       {
> > -       gcc_assert (TARGET_3DNOW);
> > -       operands[2] = GEN_INT (3);
> > +       gcc_assert (TARGET_MOVRS);
> > +       FAIL;
> >       }
> >      }
> >  })
> > --- gcc/config/i386/i386-expand.cc.jj 2024-11-26 09:37:56.019577128 +0100
> > +++ gcc/config/i386/i386-expand.cc    2024-11-26 18:03:04.095841152 +0100
> > @@ -14222,7 +14222,7 @@ ix86_expand_builtin (tree exp, rtx targe
> >
> >       if (INTVAL (op3) == 1)
> >         {
> > -         if (INTVAL (op2) < 2 || INTVAL (op2) > 3)
> > +         if (!IN_RANGE (INTVAL (op2), 2, 3))
> >             {
> >               error ("invalid third argument");
> >               return const0_rtx;
> > @@ -14252,14 +14252,16 @@ ix86_expand_builtin (tree exp, rtx targe
> >               op0 = copy_addr_to_reg (op0);
> >             }
> >
> > -         if (INTVAL (op2) < 0 || INTVAL (op2) > 3)
> > +         if (!IN_RANGE (INTVAL (op2), 0, 3))
> >             {
> >               warning (0, "invalid third argument to %<__builtin_ia32_prefetch%>; using zero");
> >               op2 = const0_rtx;
> >             }
> >
> > -         if (TARGET_3DNOW || TARGET_PREFETCH_SSE
> > -             || TARGET_PRFCHW)
> > +         if (TARGET_3DNOW
> > +             || TARGET_PREFETCH_SSE
> > +             || TARGET_PRFCHW
> > +             || TARGET_MOVRS)
> >             emit_insn (gen_prefetch (op0, op1, op2));
> >           else if (!MEM_P (op0) && side_effects_p (op0))
> >             /* Don't do anything with direct references to volatile memory,
> > --- gcc/config/alpha/alpha.md.jj      2024-11-18 09:05:00.210283154 +0100
> > +++ gcc/config/alpha/alpha.md 2024-11-26 17:35:34.620915135 +0100
> > @@ -5171,7 +5171,7 @@ (define_insn "prefetch"
> >      }
> >    };
> >
> > -  bool write = INTVAL (operands[1]) != 0;
> > +  bool write = (INTVAL (operands[1]) & 1) != 0;
> >    bool lru = INTVAL (operands[2]) != 0;
> >
> >    return alt[write][lru];
> > --- gcc/config/mips/mips.md.jj        2024-09-03 16:48:01.609846676 +0200
> > +++ gcc/config/mips/mips.md   2024-11-26 17:40:24.016866697 +0100
> > @@ -7395,6 +7395,8 @@ (define_insn "prefetch"
> >        else
> >       return "lw\t$0,%a0";
> >      }
> > +  if (operands[1] == const2_rtx)
> > +    operands[1] = const0_rtx;
> >    /* Loongson ext2 implementation pref instructions.  */
> >    if (TARGET_LOONGSON_EXT2)
> >      {
> > --- gcc/config/arc/arc.md.jj  2024-01-03 11:51:48.947489474 +0100
> > +++ gcc/config/arc/arc.md     2024-11-26 17:37:17.131481085 +0100
> > @@ -5439,9 +5439,9 @@ (define_insn "prefetch_1"
> >            (match_operand:SI 2 "const_int_operand" "n"))]
> >    "TARGET_HS"
> >    {
> > -   if (INTVAL (operands[1]))
> > +    if ((INTVAL (operands[1]) & 1) != 0)
> >        return "prefetchw [%0]";
> > -   else
> > +    else
> >        return "prefetch [%0]";
> >    }
> >    [(set_attr "type" "load")
> > @@ -5454,9 +5454,9 @@ (define_insn "prefetch_2"
> >            (match_operand:SI 3 "const_int_operand" "n,n,n"))]
> >    "TARGET_HS"
> >    {
> > -   if (INTVAL (operands[2]))
> > +    if ((INTVAL (operands[2]) & 1) != 0)
> >        return "prefetchw\\t[%0, %1]";
> > -   else
> > +    else
> >        return "prefetch\\t[%0, %1]";
> >    }
> >    [(set_attr "type" "load")
> > @@ -5468,10 +5468,10 @@ (define_insn "prefetch_3"
> >            (match_operand:SI 2 "const_int_operand" "n"))]
> >    "TARGET_HS"
> >    {
> > -   operands[0] = gen_rtx_MEM (SImode, operands[0]);
> > -   if (INTVAL (operands[1]))
> > +    operands[0] = gen_rtx_MEM (SImode, operands[0]);
> > +    if ((INTVAL (operands[1]) & 1) != 0)
> >        return "prefetchw%U0\\t%0";
> > -   else
> > +    else
> >        return "prefetch%U0\\t%0";
> >     }
> >    [(set_attr "type" "load")
> > --- gcc/config/riscv/riscv.md.jj      2024-11-20 09:29:17.965421565 +0100
> > +++ gcc/config/riscv/riscv.md 2024-11-26 17:41:49.684668268 +0100
> > @@ -4219,7 +4219,8 @@ (define_insn "prefetch"
> >  {
> >    switch (INTVAL (operands[1]))
> >    {
> > -    case 0: return TARGET_ZIHINTNTL ? "%L2prefetch.r\t%a0" : "prefetch.r\t%a0";
> > +    case 0:
> > +    case 2: return TARGET_ZIHINTNTL ? "%L2prefetch.r\t%a0" : "prefetch.r\t%a0";
> >      case 1: return TARGET_ZIHINTNTL ? "%L2prefetch.w\t%a0" : "prefetch.w\t%a0";
> >      default: gcc_unreachable ();
> >    }
> > --- gcc/config/loongarch/loongarch.md.jj      2024-10-08 09:54:47.395484057 +0200
> > +++ gcc/config/loongarch/loongarch.md 2024-11-26 17:38:56.063097105 +0100
> > @@ -4091,7 +4091,8 @@ (define_insn "prefetch"
> >  {
> >    switch (INTVAL (operands[1]))
> >    {
> > -    case 0: return "preld\t0,%a0";
> > +    case 0:
> > +    case 2: return "preld\t0,%a0";
> >      case 1: return "preld\t8,%a0";
> >      default: gcc_unreachable ();
> >    }
> > --- gcc/config/sparc/sparc.md.jj      2024-11-25 09:32:32.494140049 +0100
> > +++ gcc/config/sparc/sparc.md 2024-11-26 17:44:24.705499648 +0100
> > @@ -7832,9 +7832,9 @@ (define_insn "prefetch_64"
> >    int read_or_write = INTVAL (operands[1]);
> >    int locality = INTVAL (operands[2]);
> >
> > -  gcc_assert (read_or_write == 0 || read_or_write == 1);
> > -  gcc_assert (locality >= 0 && locality < 4);
> > -  return prefetch_instr [read_or_write][locality == 0 ? 0 : 1];
> > +  gcc_assert (IN_RANGE (read_or_write, 0, 2));
> > +  gcc_assert (IN_RANGE (locality, 0, 3));
> > +  return prefetch_instr [read_or_write & 1][locality == 0 ? 0 : 1];
> >  }
> >    [(set_attr "type" "load")
> >     (set_attr "subtype" "prefetch")])
> > @@ -7858,9 +7858,9 @@ (define_insn "prefetch_32"
> >    int read_or_write = INTVAL (operands[1]);
> >    int locality = INTVAL (operands[2]);
> >
> > -  gcc_assert (read_or_write == 0 || read_or_write == 1);
> > -  gcc_assert (locality >= 0 && locality < 4);
> > -  return prefetch_instr [read_or_write][locality == 0 ? 0 : 1];
> > +  gcc_assert (IN_RANGE (read_or_write, 0, 2));
> > +  gcc_assert (IN_RANGE (locality, 0, 3));
> > +  return prefetch_instr [read_or_write & 1][locality == 0 ? 0 : 1];
> >  }
> >    [(set_attr "type" "load")
> >     (set_attr "subtype" "prefetch")])
> > --- gcc/config/ia64/ia64.md.jj        2024-10-11 11:30:38.647392761 +0200
> > +++ gcc/config/ia64/ia64.md   2024-11-26 17:38:13.556691733 +0100
> > @@ -5041,9 +5041,9 @@ (define_insn "prefetch"
> >    int i = (INTVAL (operands[1]));
> >    int j = (INTVAL (operands[2]));
> >
> > -  gcc_assert (i == 0 || i == 1);
> > -  gcc_assert (j >= 0 && j <= 3);
> > -  return alt[i][j];
> > +  gcc_assert (IN_RANGE (i, 0, 2));
> > +  gcc_assert (IN_RANGE (j, 0, 3));
> > +  return alt[i & 1][j];
> >  }
> >    [(set_attr "itanium_class" "lfetch")])
> >
> > --- gcc/config/pa/pa.md.jj    2024-11-26 09:37:56.074576356 +0100
> > +++ gcc/config/pa/pa.md       2024-11-26 17:41:11.558201627 +0100
> > @@ -10354,7 +10354,7 @@ (define_insn "prefetch_20"
> >        "ldd 0(%0),%%r0"
> >      }
> >    };
> > -  int read_or_write = INTVAL (operands[1]) == 0 ? 0 : 1;
> > +  int read_or_write = INTVAL (operands[1]) & 1;
> >    int locality = INTVAL (operands[2]) == 0 ? 0 : 1;
> >
> >    return instr [locality][read_or_write];
> > --- gcc/config/aarch64/aarch64.md.jj  2024-11-21 09:32:35.264366968 +0100
> > +++ gcc/config/aarch64/aarch64.md     2024-11-26 17:34:29.648824047 +0100
> > @@ -1020,7 +1020,7 @@ (define_insn "prefetch"
> >         the address into a DImode MEM so that aarch64_print_operand knows
> >         how to print it.  */
> >      operands[0] = gen_rtx_MEM (DImode, operands[0]);
> > -    return pftype[INTVAL(operands[1])][locality];
> > +    return pftype[INTVAL (operands[1]) & 1][locality];
> >    }
> >    [(set_attr "type" "load_4")]
> >  )
> > --- gcc/config/rs6000/rs6000.md.jj    2024-11-21 09:32:35.411364879 +0100
> > +++ gcc/config/rs6000/rs6000.md       2024-11-26 17:42:30.845092465 +0100
> > @@ -14353,14 +14353,14 @@ (define_insn "prefetch"
> >
> >    if (REG_P (operands[0]))
> >      {
> > -      if (INTVAL (operands[1]) == 0)
> > +      if (INTVAL (operands[1]) != 1)
> >          return inst_select ? "dcbt 0,%0" : "dcbt 0,%0,16";
> >        else
> >          return inst_select ? "dcbtst 0,%0" : "dcbtst 0,%0,16";
> >      }
> >    else
> >      {
> > -      if (INTVAL (operands[1]) == 0)
> > +      if (INTVAL (operands[1]) != 1)
> >          return inst_select ? "dcbt %a0" : "dcbt %a0,16";
> >        else
> >          return inst_select ? "dcbtst %a0" : "dcbtst %a0,16";
> > --- gcc/testsuite/gcc.dg/builtin-prefetch-1.c.jj      2024-11-01 19:32:19.177964137 +0100
> > +++ gcc/testsuite/gcc.dg/builtin-prefetch-1.c 2024-11-26 17:56:00.493766587 +0100
> > @@ -23,6 +23,10 @@ good (int *p)
> >    __builtin_prefetch (p, 1, 1);
> >    __builtin_prefetch (p, 1, 2);
> >    __builtin_prefetch (p, 1, 3);
> > +  __builtin_prefetch (p, 2, 0);
> > +  __builtin_prefetch (p, 2, 1);
> > +  __builtin_prefetch (p, 2, 2);
> > +  __builtin_prefetch (p, 2, 3);
> >  }
> >
> >  void
> > --- gcc/testsuite/gcc.target/i386/pr117608-1.c.jj     2024-11-26 18:03:34.775411999 +0100
> > +++ gcc/testsuite/gcc.target/i386/pr117608-1.c        2024-11-26 18:22:08.573837662 +0100
> > @@ -0,0 +1,14 @@
> > +/* PR target/117608 */
> > +/* { dg-do compile } */
> > +/* { dg-options "-mno-movrs" } */
> > +
> > +int i;
> > +
> > +void
> > +foo (void)
> > +{
> > +  __builtin_ia32_prefetch (&i, 2, 0, 0);
> > +  __builtin_ia32_prefetch (&i, 2, 1, 0);
> > +  __builtin_ia32_prefetch (&i, 2, 2, 0);
> > +  __builtin_ia32_prefetch (&i, 2, 3, 0);
> > +}
> > --- gcc/testsuite/gcc.target/i386/pr117608-2.c.jj     2024-11-26 18:20:47.385972882 +0100
> > +++ gcc/testsuite/gcc.target/i386/pr117608-2.c        2024-11-26 18:21:27.995405054 +0100
> > @@ -0,0 +1,14 @@
> > +/* PR target/117608 */
> > +/* { dg-do compile } */
> > +/* { dg-options "-mno-mmx -mno-sse -mmovrs" } */
> > +
> > +int i;
> > +
> > +void
> > +foo (void)
> > +{
> > +  __builtin_ia32_prefetch (&i, 2, 0, 0);
> > +  __builtin_ia32_prefetch (&i, 2, 1, 0);
> > +  __builtin_ia32_prefetch (&i, 2, 2, 0);
> > +  __builtin_ia32_prefetch (&i, 2, 3, 0);
> > +}
> >
> >       Jakub
> >
> >
>
> --
> 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)