| Message ID | 97c0d824fa5aeaee52a825da7f7a17ae8616c5ab.1691636916.git.research_trasio@irq.a4lg.com |
|---|---|
| State | New |
| Headers | show |
| Series | RISC-V: Make "prefetch.i" built-in usable | expand |
On 8/9/23 21:10, Tsukasa OI via Gcc-patches wrote: > From: Tsukasa OI <research_trasio@irq.a4lg.com> > > 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_<mode>): 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_<mode>" > - [(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
On 2023/08/29 6:20, Jeff Law wrote: > > > On 8/9/23 21:10, Tsukasa OI via Gcc-patches wrote: >> From: Tsukasa OI <research_trasio@irq.a4lg.com> >> >> 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_<mode>): 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_<mode>" >> - [(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
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 <research_trasio@irq.a4lg.com> >>> >>> 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_<mode>): 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_<mode>" >>> - [(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
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_<mode>" - [(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(&foo); + __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(&foo); + __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 } } */ +/* { dg-final { scan-assembler-times "prefetch.w" 4 } } */
From: Tsukasa OI <research_trasio@irq.a4lg.com> 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_<mode>): 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(-)