8320500: [vectorapi] RISC-V: Optimize vector math operations with SLEEF #21083
Conversation
|
👋 Welcome back mli! A progress list of the required criteria for merging this PR into |
|
Hi @magicus , could you please have a look at the make part? Thanks! |
|
❗ This change is not yet ready to be integrated. |
|
@Hamlin-Li The following labels will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command. |
| EXTRA_SRC := libsleef/generated, \ | ||
| DISABLED_WARNINGS_gcc := unused-function sign-compare tautological-compare ignored-qualifiers, \ | ||
| DISABLED_WARNINGS_clang := unused-function sign-compare tautological-compare ignored-qualifiers, \ | ||
| CFLAGS := $(CFLAGS_JDKLIB) -O3 -march=rv64gcv, \ |
There was a problem hiding this comment.
I think we prefer using the C_O_FLAG_* variables instead of explicitly specifying -O3.
There was a problem hiding this comment.
Thanks, do you mean something like below? I'll fix it.
CFLAGS := $(CFLAGS_JDKLIB) $(C_O_FLAG_HI) -march=rv64gcv, \
There was a problem hiding this comment.
Sorry, I had to remind myself of how this works. We actually set this as a separate parameter on the Setup macro: OPTIMIZATION := HIGH
There was a problem hiding this comment.
Thanks. I'm sorry too, I'm not familiar with the build system.
What you expected could be something like below?
diff --git a/make/modules/jdk.incubator.vector/Lib.gmk b/make/modules/jdk.incubator.vector/Lib.gmk
index 5e52277919a..c6c6103a301 100644
--- a/make/modules/jdk.incubator.vector/Lib.gmk
+++ b/make/modules/jdk.incubator.vector/Lib.gmk
@@ -41,11 +41,12 @@ endif
ifeq ($(call isTargetOs, linux)+$(call isTargetCpu, riscv64)+$(INCLUDE_COMPILER2), true+true+true)
$(eval $(call SetupJdkLibrary, BUILD_LIBSLEEF, \
NAME := sleef, \
+ OPTIMIZATION := HIGH, \
SRC := libsleef/lib, \
EXTRA_SRC := libsleef/generated, \
DISABLED_WARNINGS_gcc := unused-function sign-compare tautological-compare ignored-qualifiers, \
DISABLED_WARNINGS_clang := unused-function sign-compare tautological-compare ignored-qualifiers, \
- CFLAGS := $(CFLAGS_JDKLIB) -O3 -march=rv64gcv, \
+ CFLAGS := $(CFLAGS_JDKLIB) -march=rv64gcv, \
LDFLAGS := $(LDFLAGS_JDKLIB) \
$(call SET_SHARED_LIBRARY_ORIGIN), \
LIBS := $(JDKLIB_LIBS) \
Hi,
Can you help to review this patch?
Thanks!
This patch is based on #20781 which added the sleef source (in particular the generated sleef inline headers). We use sleef api to vectorize the math operations in vector api.
On machine with vector intrinsic support on riscv (e.g. gcc 14+) it will generate libsleef.so with the bridge functions to sleef api, otherwise without the bridge functions.
Test
test/jdk/jdk/incubator/vector
Performance
data on bananapi
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/21083/head:pull/21083$ git checkout pull/21083Update a local copy of the PR:
$ git checkout pull/21083$ git pull https://git.openjdk.org/jdk.git pull/21083/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 21083View PR using the GUI difftool:
$ git pr show -t 21083Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/21083.diff
Webrev
Link to Webrev Comment