RFR: 8330266: RISC-V: Restore frm to RoundingMode::rne after JNI [v2]
Fei Yang
fyang at openjdk.org
Wed Apr 17 03:51:59 UTC 2024
On Tue, 16 Apr 2024 11:18:58 GMT, Vladimir Kempik <vkempik at openjdk.org> wrote:
>> Great to see there is no performance degradation on the current generation of hardware. That's a great motivator to get this change in with the branch as we already know that the branch is going to bring a performance improvement on next/other generations of hardware.
>
> Even when showing middle finger to branch predictor:
>
> unsigned char prbarray[512];
> static int setupme()
> {
> srand(time(NULL));
> for(int i = 0; i < 512; i++)
> {
> int r = rand()%15;
> prbarray[i]= (r > 10) ? (r-10) : 0;
> }
>
> return 1;
> }
>
> EXPORT void func() {
> static int setup = 0;
> static unsigned long counter = 0;
> if(!setup) setup = setupme();
> int x = prbarray[counter++%512];
> asm
> ("csrw fcsr,%0\n\t"
> :
> : "r" (x)
> );
> }
>
>
> results are still equal between branchless and branchful versions
>
> I'm ok with this change, buts lets wait for yet another opinion: do we accept such "future proof" changes or not
I don't have a strong opinion on this. This code will only be enabled by the user on command line in case we are calling some buggy external libraries which may corrupt the FP control register. It won't make a difference for the most normal cases. My local tests show that `frrm` is 3x faster than `fsrmi` on sifive/u74. So it does make sense to have the branch check on platforms like this. It should not be a big issue to other more advanced platforms with a good BranchPredictor.
Suggesion: `beqz(tmp, skip_fsrmi); // Only reset FRM if it's wrong`
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18785#discussion_r1568178068
More information about the hotspot-dev
mailing list