8299525: RISC-V: Add backend support for half float conversion intrinsics #11828

yadongw · 2023-01-03T12:08:47Z

This patch adds RISC-V backend support for library intrinsics that implement conversions between half-precision and single-precision floats by using RISC-V Zfhmin Extension, which was already ratified by November 2021 (https://wiki.riscv.org/display/HOME/Recently+Ratified+Extensions) and one of RVA22U64 Mandatory Extensions (https://github.com/riscv/riscv-profiles/blob/main/profiles.adoc#rva22-profiles).

The C2 output for PrintOptoAssembly:
0dc B10: # out( B33 B11 ) <- in( B9 ) Freq: 1.99802
0dc + flw F1, [R29, #16] # float, #@LoadF
0e0 + fcvt.h.s F0, F1 #@convF2HF_reg_reg
fmv.x.h R8, F0 #@convF2HF_reg_reg

0dc B10: # out( B33 B11 ) <- in( B9 ) Freq: 1.99801
0dc + lh R12, [R11, #16] # short, #@loads
0e0 + fmv.h.x F0, R12 #@convHF2F_reg_reg
fcvt.s.h F1, F0 #@convHF2F_reg_reg

We don't have any hardware supporting yet, so ran the following benchmarks in Qemu for unreliable reference:

VM options: -XX:+UnlockExperimentalVMOptions -XX:-UseZfhmin
Benchmark (size) Mode Samples Score Score error Units
o.s.Fp16ConversionBenchmark.float16ToFloat 2048 thrpt 15 44.523 0.116 ops/ms
o.s.Fp16ConversionBenchmark.float16ToFloatMemory 2048 thrpt 15 8379.835 27.309 ops/ms
o.s.Fp16ConversionBenchmark.floatToFloat16 2048 thrpt 15 7.370 0.028 ops/ms
o.s.Fp16ConversionBenchmark.floatToFloat16Memory 2048 thrpt 15 11292.278 11.962 ops/ms

VM options: -XX:+UnlockExperimentalVMOptions -XX:+UseZfhmin
Benchmark (size) Mode Samples Score Score error Units
o.s.Fp16ConversionBenchmark.float16ToFloat 2048 thrpt 15 12.357 0.153 ops/ms
o.s.Fp16ConversionBenchmark.float16ToFloatMemory 2048 thrpt 15 10213.944 69.222 ops/ms
o.s.Fp16ConversionBenchmark.floatToFloat16 2048 thrpt 15 11.728 0.067 ops/ms
o.s.Fp16ConversionBenchmark.floatToFloat16Memory 2048 thrpt 15 15008.550 13.917 ops/ms

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8299525: RISC-V: Add backend support for half float conversion intrinsics

Reviewers

Fei Yang (@RealFYang - Reviewer)
Feilong Jiang (@feilongjiang - Author)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/11828/head:pull/11828
$ git checkout pull/11828

Update a local copy of the PR:
$ git checkout pull/11828
$ git pull https://git.openjdk.org/jdk pull/11828/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 11828

View PR using the GUI difftool:
$ git pr show -t 11828

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/11828.diff

…sics

bridgekeeper · 2023-01-03T12:10:26Z

👋 Welcome back yadongwang! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2023-01-03T12:13:18Z

@yadongw The following label will be automatically applied to this pull request:

hotspot-compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2023-01-03T12:16:44Z

Webrevs

RealFYang · 2023-01-04T12:55:45Z

src/hotspot/cpu/riscv/riscv.ad

+  %}
+
+  ins_encode %{
+      __ fmv_h_x($tmp$$FloatRegister, $src$$Register);


The indentation here is two spaces instead of four spaces.

RealFYang · 2023-01-04T12:55:59Z

src/hotspot/cpu/riscv/riscv.ad

+  %}
+
+  ins_encode %{
+      __ fcvt_h_s($tmp$$FloatRegister, $src$$FloatRegister);


Same issue here.

feilongjiang · 2023-01-04T12:55:49Z

src/hotspot/cpu/riscv/riscv.ad

+      __ fcvt_s_h($dst$$FloatRegister, $tmp$$FloatRegister);
+  %}
+
+  ins_pipe(fp_f2d);


According to the type of src and dst, we can use fp_i2f as ins_pipe.

feilongjiang · 2023-01-04T12:56:42Z

src/hotspot/cpu/riscv/riscv.ad

+      __ fmv_x_h($dst$$Register, $tmp$$FloatRegister);
+  %}
+
+  ins_pipe(fp_f2d);


Same here, fp_f2i is better.

src/hotspot/cpu/riscv/globals_riscv.hpp

feilongjiang · 2023-01-04T13:09:10Z

Zfhmin is a mandatory extension of RVA22U64 Profile [1], we can enable Zfhmin when UseRVA22U64 is true:

jdk/src/hotspot/cpu/riscv/vm_version_riscv.cpp

Lines 51 to 74 in 82deb5c

    
           // https://github.com/riscv/riscv-profiles/blob/main/profiles.adoc#rva22-profiles 
        
           if (UseRVA22U64) { 
        
             if (FLAG_IS_DEFAULT(UseRVC)) { 
        
               FLAG_SET_DEFAULT(UseRVC, true); 
        
             } 
        
             if (FLAG_IS_DEFAULT(UseZba)) { 
        
               FLAG_SET_DEFAULT(UseZba, true); 
        
             } 
        
             if (FLAG_IS_DEFAULT(UseZbb)) { 
        
               FLAG_SET_DEFAULT(UseZbb, true); 
        
             } 
        
             if (FLAG_IS_DEFAULT(UseZic64b)) { 
        
               FLAG_SET_DEFAULT(UseZic64b, true); 
        
             } 
        
             if (FLAG_IS_DEFAULT(UseZicbom)) { 
        
               FLAG_SET_DEFAULT(UseZicbom, true); 
        
             } 
        
             if (FLAG_IS_DEFAULT(UseZicbop)) { 
        
               FLAG_SET_DEFAULT(UseZicbop, true); 
        
             } 
        
             if (FLAG_IS_DEFAULT(UseZicboz)) { 
        
               FLAG_SET_DEFAULT(UseZicboz, true); 
        
             } 
        
           }

https://github.com/riscv/riscv-profiles/blob/main/profiles.adoc#612-rva22u64-mandatory-extensions

feilongjiang

LGTM

RealFYang

Updated change looks good.

openjdk · 2023-01-06T13:47:30Z

@yadongw This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8299525: RISC-V: Add backend support for half float conversion intrinsics

Reviewed-by: fyang, fjiang

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 112 new commits pushed to the master branch:

b7eb4e2: 8297306: Incorrect brackets in Javadoc for a constructor of IteratorSpliterator
a638663: 8255119: Monitor::wait takes signed integer as timeout
4395578: Merge
d49851a: 8299689: Make use of JLine for Console as "opt-in"
f79b3d4: 6381945: (cal) Japanese calendar unit test system should avoid multiple static imports
f36f135: 8299501: Usage of constructors of primitive wrapper classes should be avoided in java.util API docs
b8852f6: 8299502: Usage of constructors of primitive wrapper classes should be avoided in javax.xml API docs
679e485: 8043251: Bogus javac error: required: no arguments, found: no arguments
cd10c72: 8299500: Usage of constructors of primitive wrapper classes should be avoided in java.text API docs
bfd5971: 8299757: Update JCov version to 3.0.14
... and 102 more: https://git.openjdk.org/jdk/compare/6f85a9c9a8ea3f76575acb4964cd80219822f073...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@RealFYang) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

yadongw · 2023-01-10T02:06:15Z

/integrate

openjdk · 2023-01-10T02:07:36Z

@yadongw
Your change (at version 271f6d6) is now ready to be sponsored by a Committer.

RealFYang · 2023-01-10T02:42:15Z

/sponsor

openjdk · 2023-01-10T02:43:23Z

Going to push as commit 3a66737.
Since your change was applied there have been 112 commits pushed to the master branch:

b7eb4e2: 8297306: Incorrect brackets in Javadoc for a constructor of IteratorSpliterator
a638663: 8255119: Monitor::wait takes signed integer as timeout
4395578: Merge
d49851a: 8299689: Make use of JLine for Console as "opt-in"
f79b3d4: 6381945: (cal) Japanese calendar unit test system should avoid multiple static imports
f36f135: 8299501: Usage of constructors of primitive wrapper classes should be avoided in java.util API docs
b8852f6: 8299502: Usage of constructors of primitive wrapper classes should be avoided in javax.xml API docs
679e485: 8043251: Bogus javac error: required: no arguments, found: no arguments
cd10c72: 8299500: Usage of constructors of primitive wrapper classes should be avoided in java.text API docs
bfd5971: 8299757: Update JCov version to 3.0.14
... and 102 more: https://git.openjdk.org/jdk/compare/6f85a9c9a8ea3f76575acb4964cd80219822f073...master

Your commit was automatically rebased without conflicts.

openjdk · 2023-01-10T02:43:37Z

@RealFYang @yadongw Pushed as commit 3a66737.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

8299525: RISC-V: Add backend support for half float conversion intrin…

1faf13b

…sics

openjdk bot changed the title ~~8299525: RISC-V: Add backend support for half float conversion intrin…~~ 8299525: RISC-V: Add backend support for half float conversion intrinsics Jan 3, 2023

openjdk bot added the rfr Pull request is ready for review label Jan 3, 2023

openjdk bot added the hotspot-compiler [email protected] label Jan 3, 2023

RealFYang suggested changes Jan 4, 2023

View reviewed changes

feilongjiang reviewed Jan 4, 2023

View reviewed changes

yadongw added 2 commits January 5, 2023 22:37

change Zfh to Zfhmin

8a08bfd

fix

271f6d6

feilongjiang approved these changes Jan 6, 2023

View reviewed changes

RealFYang approved these changes Jan 6, 2023

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jan 6, 2023

openjdk bot added the sponsor Pull request is ready to be sponsored label Jan 10, 2023

openjdk bot added the integrated Pull request has been integrated label Jan 10, 2023

openjdk bot closed this Jan 10, 2023

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Jan 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8299525: RISC-V: Add backend support for half float conversion intrinsics #11828

8299525: RISC-V: Add backend support for half float conversion intrinsics #11828

yadongw commented Jan 3, 2023 •

edited by openjdk bot

Loading

bridgekeeper bot commented Jan 3, 2023

openjdk bot commented Jan 3, 2023

mlbridge bot commented Jan 3, 2023 •

edited

Loading

RealFYang Jan 4, 2023

RealFYang Jan 4, 2023

feilongjiang Jan 4, 2023

feilongjiang Jan 4, 2023

feilongjiang commented Jan 4, 2023

feilongjiang left a comment

RealFYang left a comment

openjdk bot commented Jan 6, 2023 •

edited

Loading

yadongw commented Jan 10, 2023

openjdk bot commented Jan 10, 2023

RealFYang commented Jan 10, 2023

openjdk bot commented Jan 10, 2023

openjdk bot commented Jan 10, 2023

8299525: RISC-V: Add backend support for half float conversion intrinsics #11828

8299525: RISC-V: Add backend support for half float conversion intrinsics #11828

Conversation

yadongw commented Jan 3, 2023 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Jan 3, 2023

openjdk bot commented Jan 3, 2023

mlbridge bot commented Jan 3, 2023 • edited Loading

Webrevs

RealFYang Jan 4, 2023

Choose a reason for hiding this comment

RealFYang Jan 4, 2023

Choose a reason for hiding this comment

feilongjiang Jan 4, 2023

Choose a reason for hiding this comment

feilongjiang Jan 4, 2023

Choose a reason for hiding this comment

feilongjiang commented Jan 4, 2023

feilongjiang left a comment

Choose a reason for hiding this comment

RealFYang left a comment

Choose a reason for hiding this comment

openjdk bot commented Jan 6, 2023 • edited Loading

yadongw commented Jan 10, 2023

openjdk bot commented Jan 10, 2023

RealFYang commented Jan 10, 2023

openjdk bot commented Jan 10, 2023

openjdk bot commented Jan 10, 2023

yadongw commented Jan 3, 2023 •

edited by openjdk bot

Loading

mlbridge bot commented Jan 3, 2023 •

edited

Loading

openjdk bot commented Jan 6, 2023 •

edited

Loading