https://gcc.gnu.org/g:f7bbdf7e4acf9c90f51d24db9a3c911c49169ab6
commit r15-5771-gf7bbdf7e4acf9c90f51d24db9a3c911c49169ab6 Author: Jakub Jelinek <ja...@redhat.com> Date: Fri Nov 29 10:17:07 2024 +0100 __builtin_prefetch fixes [PR117608] 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. 2024-11-29 Jakub Jelinek <ja...@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. Diff: --- gcc/config/aarch64/aarch64.md | 2 +- gcc/config/alpha/alpha.md | 2 +- gcc/config/arc/arc.md | 14 +++++------ gcc/config/i386/i386-expand.cc | 10 ++++---- gcc/config/i386/i386.md | 37 +++++++++++++----------------- gcc/config/ia64/ia64.md | 6 ++--- gcc/config/loongarch/loongarch.md | 3 ++- gcc/config/mips/mips.md | 2 ++ gcc/config/pa/pa.md | 2 +- gcc/config/riscv/riscv.md | 3 ++- gcc/config/rs6000/rs6000.md | 4 ++-- gcc/config/sparc/sparc.md | 12 +++++----- gcc/doc/extend.texi | 8 ++++--- gcc/testsuite/gcc.dg/builtin-prefetch-1.c | 4 ++++ gcc/testsuite/gcc.target/i386/pr117608-1.c | 14 +++++++++++ gcc/testsuite/gcc.target/i386/pr117608-2.c | 14 +++++++++++ 16 files changed, 86 insertions(+), 51 deletions(-) diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index 711e9adc7575..87ba18620ada 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -1020,7 +1020,7 @@ 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")] ) diff --git a/gcc/config/alpha/alpha.md b/gcc/config/alpha/alpha.md index e57a9d31e013..22ea4db057c3 100644 --- a/gcc/config/alpha/alpha.md +++ b/gcc/config/alpha/alpha.md @@ -5171,7 +5171,7 @@ } }; - bool write = INTVAL (operands[1]) != 0; + bool write = (INTVAL (operands[1]) & 1) != 0; bool lru = INTVAL (operands[2]) != 0; return alt[write][lru]; diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md index 9004b6085a23..1fb7cf5f71de 100644 --- a/gcc/config/arc/arc.md +++ b/gcc/config/arc/arc.md @@ -5439,9 +5439,9 @@ archs4x, archs4xd" (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 @@ archs4x, archs4xd" (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 @@ archs4x, archs4xd" (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") diff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc index 2eb619725047..6d25477841a9 100644 --- a/gcc/config/i386/i386-expand.cc +++ b/gcc/config/i386/i386-expand.cc @@ -14222,7 +14222,7 @@ ix86_expand_builtin (tree exp, rtx target, rtx subtarget, 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 target, rtx subtarget, 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, diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index 8eb9cb682b11..ffbb10730c0b 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -28595,8 +28595,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]); @@ -28609,7 +28608,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); @@ -28617,33 +28616,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; } } }) diff --git a/gcc/config/ia64/ia64.md b/gcc/config/ia64/ia64.md index d485acc0ea86..c533a9c106c6 100644 --- a/gcc/config/ia64/ia64.md +++ b/gcc/config/ia64/ia64.md @@ -5041,9 +5041,9 @@ 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")]) diff --git a/gcc/config/loongarch/loongarch.md b/gcc/config/loongarch/loongarch.md index bd0825002387..a65cc1de2d2e 100644 --- a/gcc/config/loongarch/loongarch.md +++ b/gcc/config/loongarch/loongarch.md @@ -4091,7 +4091,8 @@ { 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 (); } diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md index f147667d63a8..dff7ded2ab28 100644 --- a/gcc/config/mips/mips.md +++ b/gcc/config/mips/mips.md @@ -7395,6 +7395,8 @@ else return "lw\t$0,%a0"; } + if (operands[1] == const2_rtx) + operands[1] = const0_rtx; /* Loongson ext2 implementation pref instructions. */ if (TARGET_LOONGSON_EXT2) { diff --git a/gcc/config/pa/pa.md b/gcc/config/pa/pa.md index c4441d8b91d0..61447c0db5c5 100644 --- a/gcc/config/pa/pa.md +++ b/gcc/config/pa/pa.md @@ -10354,7 +10354,7 @@ add,l %2,%3,%3\;bv,n %%r0(%3)" "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]; diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md index eb5cd6fbe82d..f7a6db354af5 100644 --- a/gcc/config/riscv/riscv.md +++ b/gcc/config/riscv/riscv.md @@ -4219,7 +4219,8 @@ { 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 (); } diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md index 95be36d5a726..edccd7817ae1 100644 --- a/gcc/config/rs6000/rs6000.md +++ b/gcc/config/rs6000/rs6000.md @@ -14353,14 +14353,14 @@ 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"; diff --git a/gcc/config/sparc/sparc.md b/gcc/config/sparc/sparc.md index 0de9a7b6394b..5fd18b3ca27c 100644 --- a/gcc/config/sparc/sparc.md +++ b/gcc/config/sparc/sparc.md @@ -7832,9 +7832,9 @@ 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 @@ 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")]) diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index f0f007bb746c..c55991df8f10 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -15711,9 +15711,11 @@ be in the cache by the time it is accessed. 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 diff --git a/gcc/testsuite/gcc.dg/builtin-prefetch-1.c b/gcc/testsuite/gcc.dg/builtin-prefetch-1.c index aadbf144cfe6..a24c5f7ebc6c 100644 --- a/gcc/testsuite/gcc.dg/builtin-prefetch-1.c +++ b/gcc/testsuite/gcc.dg/builtin-prefetch-1.c @@ -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 diff --git a/gcc/testsuite/gcc.target/i386/pr117608-1.c b/gcc/testsuite/gcc.target/i386/pr117608-1.c new file mode 100644 index 000000000000..03c1403dfea0 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr117608-1.c @@ -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); +} diff --git a/gcc/testsuite/gcc.target/i386/pr117608-2.c b/gcc/testsuite/gcc.target/i386/pr117608-2.c new file mode 100644 index 000000000000..798484895d65 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr117608-2.c @@ -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); +}