Skip to content

Recognize @IntrinsicCandidate java.lang.Math.multiplyHigh - #17861

Merged
jdmpapin merged 1 commit into
eclipse-openj9:masterfrom
jmesyou:intrinsics/multiplyHigh
Aug 31, 2023
Merged

Recognize @IntrinsicCandidate java.lang.Math.multiplyHigh#17861
jdmpapin merged 1 commit into
eclipse-openj9:masterfrom
jmesyou:intrinsics/multiplyHigh

Conversation

@jmesyou

@jmesyou jmesyou commented Jul 26, 2023

Copy link
Copy Markdown
Contributor

java.lang.Math.multiplyHigh has been an intrinsic candidate in the Java class library since JDK17. There is a single opcode which implements the functionality of multiplyHigh. This provides an opportunity to simplify calls to multiplyHigh much like other Math methods (abs, min, max, etc).

This commit adds the following:

  • The recognition of java.lang.Math.multiplyHigh as a recognized method.
  • The replacement of calls to java.lang.Math.multiplyHigh with a single lmulh node as part of RecognizedCallTransformer

@jmesyou
jmesyou force-pushed the intrinsics/multiplyHigh branch from b41e037 to c61c2df Compare July 26, 2023 21:00
@jmesyou
jmesyou requested a review from jdmpapin July 26, 2023 21:02

@jdmpapin jdmpapin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were you able to observe the transformation occurring? I think both of the below two problems would independently prevent it

Comment thread runtime/compiler/env/j9method.cpp Outdated
Comment thread runtime/compiler/optimizer/J9RecognizedCallTransformer.cpp Outdated
@jmesyou

jmesyou commented Jul 26, 2023

Copy link
Copy Markdown
Contributor Author

You are right, I was wondering why I didn't see this transform happening after I opened the PR and was about to mark this WIP. I'll push the changes to enable them.

@jmesyou
jmesyou force-pushed the intrinsics/multiplyHigh branch from c61c2df to 23a477e Compare July 26, 2023 22:28
Comment thread runtime/compiler/optimizer/J9RecognizedCallTransformer.cpp Outdated
@jmesyou
jmesyou requested a review from jdmpapin July 26, 2023 22:29
@jmesyou

jmesyou commented Jul 26, 2023

Copy link
Copy Markdown
Contributor Author

I can confirm the transformation happens now!

@jmesyou

jmesyou commented Aug 1, 2023

Copy link
Copy Markdown
Contributor Author

Waiting on #17892

java.lang.Math.multiplyHigh has been an intrinsic candidate
in the Java class library since JDK17. There is a single
opcode which implements the functionality of multiplyHigh.
This provides an opportunity to simplify calls to multiplyHigh
much like other Math methods (abs, min, max, etc).

This commit adds the following:

+ The recognition of java.lang.Math.multiplyHigh as a
recognized method.
+ The replacement of calls to java.lang.Math.multiplyHigh
with a single lmulh node as part of RecognizedCallTransformer

Signed-off-by: James You <james.you@protonmail.com>
@jmesyou
jmesyou force-pushed the intrinsics/multiplyHigh branch from 23a477e to f03f364 Compare August 30, 2023 04:32
@jmesyou

jmesyou commented Aug 30, 2023

Copy link
Copy Markdown
Contributor Author

@jdmpapin , ready for another pass!

@jdmpapin

Copy link
Copy Markdown
Contributor

Jenkins test sanity all jdk21

@jdmpapin
jdmpapin merged commit 022a2a4 into eclipse-openj9:master Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants