RFR: 8347343: RISC-V: Unchecked zicntr csr reads

Robbin Ehn rehn at openjdk.org
Fri Jan 10 08:44:35 UTC 2025


On Fri, 10 Jan 2025 03:13:41 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> Hi, please consider.
>> 
>> The rdinstret/rdcycle/rdtime are really useful and would be great to add in e.g. JFR.
>> But as of now they are not used and zicntr is not yet in hwprobe therefore this patch just make sure they can't be used.
>> 
>> If we need them before hwprobe we can add a flag for manually enabling them.
>> 
>> Sanity tested.
>> 
>> Thanks!
>
> src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 1338:
> 
>> 1336: 
>> 1337: void MacroAssembler::csrr(Register Rd, unsigned csr) {
>> 1338:   assert((csr != CSR_INSTRET && csr != CSR_CYCLE && csr != CSR_TIME) || VM_Version::ext_Zicntr.enabled(), "sanity");
> 
> Personally, I perfer to check a JVM option here (Say `UseZicntr` of diagnostic type). When we have hwprobe for this extension, we can further hook up this option to `VM_Version::ext_Zicntr`. And the user could still enable on command line before that. What do you think?

As these instructions are unused, no need to enable them, I'm hesitant to add an NO-OP option.
I figured that when we add uses fom them we can add that option, if needed.

Should I really add such dummy option?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23003#discussion_r1910005242


More information about the hotspot-dev mailing list