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_tra...@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
