Re: [PATCH 1/1] RISC-V: Make "prefetch.i" built-in usable
On 8/28/23 20:09, Tsukasa OI wrote: On 2023/08/29 6:20, Jeff Law wrote: On 8/9/23 21:10, Tsukasa OI via Gcc-patches wrote: From: Tsukasa OI The "__builtin_riscv_zicbop_cbo_prefetchi" built-in function was terribly broken so that practically unusable. It emitted "prefetch.i" but with no meaningful arguments. Though incompatible, this commit completely changes the function prototype of this built-in and makes it usable. To minimize the functionality issues, it renames the built-in to "__builtin_riscv_zicbop_prefetch_i". gcc/ChangeLog: * config/riscv/riscv-cmo.def: Fix function prototype. * config/riscv/riscv.md (riscv_prefetchi_): Fix instruction prototype. Remove possible prefectch type argument * doc/extend.texi: Document __builtin_riscv_zicbop_prefetch_i. gcc/testsuite/ChangeLog: * gcc.target/riscv/cmo-zicbop-1.c: Reflect new built-in prototype. * gcc.target/riscv/cmo-zicbop-2.c: Likewise. diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md index 688fd697255b..5658c7b7e113 100644 --- a/gcc/config/riscv/riscv.md +++ b/gcc/config/riscv/riscv.md @@ -3273,9 +3273,8 @@ }) (define_insn "riscv_prefetchi_" - [(unspec_volatile:X [(match_operand:X 0 "address_operand" "r") - (match_operand:X 1 "imm5_operand" "i")] - UNSPECV_PREI)] + [(unspec_volatile:X [(match_operand:X 0 "register_operand" "r")] + UNSPECV_PREI)] "TARGET_ZICBOP" "prefetch.i\t%a0" ) What I would suggest is making a new predicate that accepts either a register or a register+offset where the offset fits in a signed 12 bit immediate. Use that for operand 0's predicate and I think this will "just work" and cover all the cases supported by the prefetch.i instruction. Jeff Seems reasonable. If we have to break the compatibility anyway, adding an offset argument is not a bad idea (though I think they will use inline assembly if a non-zero offset is required). I will try to add *optional* offset argument (with default value 0) like __builtin_speculation_safe_value built-in function in the next version. The reason I suggested creating a predicate which allowed either a reg or reg+offset was to give that level of flexibility. The inline assembly would specify an address. If the address was already in a register, great. If the address is expressable as reg+offset, also great. If the address was a symbolic operand, the right predicate+constraint should force the symbolic into a register. Jeff
Re: [PATCH 1/1] RISC-V: Make "prefetch.i" built-in usable
On 2023/08/29 6:20, Jeff Law wrote: > > > On 8/9/23 21:10, Tsukasa OI via Gcc-patches wrote: >> From: Tsukasa OI >> >> The "__builtin_riscv_zicbop_cbo_prefetchi" built-in function was terribly >> broken so that practically unusable. It emitted "prefetch.i" but with no >> meaningful arguments. >> >> Though incompatible, this commit completely changes the function >> prototype >> of this built-in and makes it usable. To minimize the functionality >> issues, >> it renames the built-in to "__builtin_riscv_zicbop_prefetch_i". >> >> gcc/ChangeLog: >> >> * config/riscv/riscv-cmo.def: Fix function prototype. >> * config/riscv/riscv.md (riscv_prefetchi_): Fix instruction >> prototype. Remove possible prefectch type argument >> * doc/extend.texi: Document __builtin_riscv_zicbop_prefetch_i. >> >> gcc/testsuite/ChangeLog: >> >> * gcc.target/riscv/cmo-zicbop-1.c: Reflect new built-in prototype. >> * gcc.target/riscv/cmo-zicbop-2.c: Likewise. >> diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md >> index 688fd697255b..5658c7b7e113 100644 >> --- a/gcc/config/riscv/riscv.md >> +++ b/gcc/config/riscv/riscv.md >> @@ -3273,9 +3273,8 @@ >> }) >> (define_insn "riscv_prefetchi_" >> - [(unspec_volatile:X [(match_operand:X 0 "address_operand" "r") >> - (match_operand:X 1 "imm5_operand" "i")] >> - UNSPECV_PREI)] >> + [(unspec_volatile:X [(match_operand:X 0 "register_operand" "r")] >> + UNSPECV_PREI)] >> "TARGET_ZICBOP" >> "prefetch.i\t%a0" >> ) > What I would suggest is making a new predicate that accepts either a > register or a register+offset where the offset fits in a signed 12 bit > immediate. Use that for operand 0's predicate and I think this will > "just work" and cover all the cases supported by the prefetch.i > instruction. > > Jeff > Seems reasonable. If we have to break the compatibility anyway, adding an offset argument is not a bad idea (though I think they will use inline assembly if a non-zero offset is required). I will try to add *optional* offset argument (with default value 0) like __builtin_speculation_safe_value built-in function in the next version. Thanks, Tsukasa
Re: [PATCH 1/1] RISC-V: Make "prefetch.i" built-in usable
On 8/9/23 21:10, Tsukasa OI via Gcc-patches wrote: From: Tsukasa OI The "__builtin_riscv_zicbop_cbo_prefetchi" built-in function was terribly broken so that practically unusable. It emitted "prefetch.i" but with no meaningful arguments. Though incompatible, this commit completely changes the function prototype of this built-in and makes it usable. To minimize the functionality issues, it renames the built-in to "__builtin_riscv_zicbop_prefetch_i". gcc/ChangeLog: * config/riscv/riscv-cmo.def: Fix function prototype. * config/riscv/riscv.md (riscv_prefetchi_): Fix instruction prototype. Remove possible prefectch type argument * doc/extend.texi: Document __builtin_riscv_zicbop_prefetch_i. gcc/testsuite/ChangeLog: * gcc.target/riscv/cmo-zicbop-1.c: Reflect new built-in prototype. * gcc.target/riscv/cmo-zicbop-2.c: Likewise. diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md index 688fd697255b..5658c7b7e113 100644 --- a/gcc/config/riscv/riscv.md +++ b/gcc/config/riscv/riscv.md @@ -3273,9 +3273,8 @@ }) (define_insn "riscv_prefetchi_" - [(unspec_volatile:X [(match_operand:X 0 "address_operand" "r") - (match_operand:X 1 "imm5_operand" "i")] - UNSPECV_PREI)] + [(unspec_volatile:X [(match_operand:X 0 "register_operand" "r")] +UNSPECV_PREI)] "TARGET_ZICBOP" "prefetch.i\t%a0" ) What I would suggest is making a new predicate that accepts either a register or a register+offset where the offset fits in a signed 12 bit immediate. Use that for operand 0's predicate and I think this will "just work" and cover all the cases supported by the prefetch.i instruction. Jeff
[PATCH 1/1] RISC-V: Make "prefetch.i" built-in usable
From: Tsukasa OI
The "__builtin_riscv_zicbop_cbo_prefetchi" built-in function was terribly
broken so that practically unusable. It emitted "prefetch.i" but with no
meaningful arguments.
Though incompatible, this commit completely changes the function prototype
of this built-in and makes it usable. To minimize the functionality issues,
it renames the built-in to "__builtin_riscv_zicbop_prefetch_i".
gcc/ChangeLog:
* config/riscv/riscv-cmo.def: Fix function prototype.
* config/riscv/riscv.md (riscv_prefetchi_): Fix instruction
prototype. Remove possible prefectch type argument
* doc/extend.texi: Document __builtin_riscv_zicbop_prefetch_i.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/cmo-zicbop-1.c: Reflect new built-in prototype.
* gcc.target/riscv/cmo-zicbop-2.c: Likewise.
---
gcc/config/riscv/riscv-cmo.def| 4 ++--
gcc/config/riscv/riscv.md | 5 ++---
gcc/doc/extend.texi | 7 +++
gcc/testsuite/gcc.target/riscv/cmo-zicbop-1.c | 8 +---
gcc/testsuite/gcc.target/riscv/cmo-zicbop-2.c | 10 ++
5 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/gcc/config/riscv/riscv-cmo.def b/gcc/config/riscv/riscv-cmo.def
index b92044dc6ff9..2286c8a25544 100644
--- a/gcc/config/riscv/riscv-cmo.def
+++ b/gcc/config/riscv/riscv-cmo.def
@@ -13,8 +13,8 @@ RISCV_BUILTIN (zero_si, "zicboz_cbo_zero",
RISCV_BUILTIN_DIRECT_NO_TARGET, RISCV
RISCV_BUILTIN (zero_di, "zicboz_cbo_zero", RISCV_BUILTIN_DIRECT_NO_TARGET,
RISCV_VOID_FTYPE_VOID_PTR, zero64),
// zicbop
-RISCV_BUILTIN (prefetchi_si, "zicbop_cbo_prefetchi", RISCV_BUILTIN_DIRECT,
RISCV_SI_FTYPE_SI, prefetchi32),
-RISCV_BUILTIN (prefetchi_di, "zicbop_cbo_prefetchi", RISCV_BUILTIN_DIRECT,
RISCV_DI_FTYPE_DI, prefetchi64),
+RISCV_BUILTIN (prefetchi_si, "zicbop_prefetch_i",
RISCV_BUILTIN_DIRECT_NO_TARGET, RISCV_VOID_FTYPE_VOID_PTR, prefetchi32),
+RISCV_BUILTIN (prefetchi_di, "zicbop_prefetch_i",
RISCV_BUILTIN_DIRECT_NO_TARGET, RISCV_VOID_FTYPE_VOID_PTR, prefetchi64),
// zbkc or zbc
RISCV_BUILTIN (clmul_si, "clmul", RISCV_BUILTIN_DIRECT, RISCV_SI_FTYPE_SI_SI,
clmul_zbkc32_or_zbc32),
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index 688fd697255b..5658c7b7e113 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -3273,9 +3273,8 @@
})
(define_insn "riscv_prefetchi_"
- [(unspec_volatile:X [(match_operand:X 0 "address_operand" "r")
- (match_operand:X 1 "imm5_operand" "i")]
- UNSPECV_PREI)]
+ [(unspec_volatile:X [(match_operand:X 0 "register_operand" "r")]
+UNSPECV_PREI)]
"TARGET_ZICBOP"
"prefetch.i\t%a0"
)
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 89c5b4ea2b20..0eb98fc89e3f 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -21575,6 +21575,13 @@ Xgnuzihintpausestate extension, which redefines the
@code{pause} instruction to
change architectural state.
@enddefbuiltin
+@defbuiltin{void __builtin_riscv_zicbop_prefetch_i (void *@var{addr})}
+Generates the @code{prefetch.i} machine instruction to instruct the hardware
+that a cache block containing @var{addr} is likely to be accessed by an
+instruction fetch in the near future.
+Available if the Zicbop extension is enabled.
+@enddefbuiltin
+
@node RISC-V Vector Intrinsics
@subsection RISC-V Vector Intrinsics
diff --git a/gcc/testsuite/gcc.target/riscv/cmo-zicbop-1.c
b/gcc/testsuite/gcc.target/riscv/cmo-zicbop-1.c
index c5d78c1763d3..0d5b58c4fadf 100644
--- a/gcc/testsuite/gcc.target/riscv/cmo-zicbop-1.c
+++ b/gcc/testsuite/gcc.target/riscv/cmo-zicbop-1.c
@@ -13,11 +13,13 @@ void foo (char *p)
__builtin_prefetch (p, 1, 3);
}
-int foo1()
+void foo1 ()
{
- return __builtin_riscv_zicbop_cbo_prefetchi(1);
+ __builtin_riscv_zicbop_prefetch_i(0);
+ __builtin_riscv_zicbop_prefetch_i();
+ __builtin_riscv_zicbop_prefetch_i((void*)0x111);
}
-/* { dg-final { scan-assembler-times "prefetch.i" 1 } } */
+/* { dg-final { scan-assembler-times "prefetch.i" 3 } } */
/* { dg-final { scan-assembler-times "prefetch.r" 4 } } */
/* { dg-final { scan-assembler-times "prefetch.w" 4 } } */
diff --git a/gcc/testsuite/gcc.target/riscv/cmo-zicbop-2.c
b/gcc/testsuite/gcc.target/riscv/cmo-zicbop-2.c
index 6576365b39ca..09655c4b8593 100644
--- a/gcc/testsuite/gcc.target/riscv/cmo-zicbop-2.c
+++ b/gcc/testsuite/gcc.target/riscv/cmo-zicbop-2.c
@@ -13,11 +13,13 @@ void foo (char *p)
__builtin_prefetch (p, 1, 3);
}
-int foo1()
+void foo1 ()
{
- return __builtin_riscv_zicbop_cbo_prefetchi(1);
+ __builtin_riscv_zicbop_prefetch_i(0);
+ __builtin_riscv_zicbop_prefetch_i();
+ __builtin_riscv_zicbop_prefetch_i((void*)0x111);
}
-/* { dg-final { scan-assembler-times "prefetch.i" 1 } } */
+/* { dg-final { scan-assembler-times "prefetch.i" 3 } } */
/* { dg-final { scan-assembler-times "prefetch.r" 4 } } */
-/* { dg-final { scan-assembler-times "prefetch.w" 4 } } */
+/* {
