8313372: [JVMCI] Export vmIntrinsics::is_intrinsic_available results to JVMCI compilers. #15133

mur47x111 · 2023-08-03T06:48:27Z

This PR exports vmIntrinsic::is_intrinsic_available, Compiler::is_intrinsic_supported, and C2Compiler::is_intrinsic_supported results to JVMCI compiler. This allows JVMCI compiler to comply with -XX:DisableIntrinsic, -XX:ControlIntrinsic, -XX:-UseXXXIntrinsic, and is essential for running test that depends on these flags, e.g., java/lang/Float/Binary16ConversionNaN that returns different result in the interpreter with -XX:DisableIntrinsic=_float16ToFloat,_floatToFloat16.
This PR also attempts to fix some of the is_intrinsic_available results. Please see the inlined comments.

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-8313372: [JVMCI] Export vmIntrinsics::is_intrinsic_available results to JVMCI compilers. (Enhancement - P4)

Reviewers

Doug Simon (@dougxc - Reviewer)
Vladimir Kozlov (@vnkozlov - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 15133

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

Using diff file

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

Webrev

Link to Webrev Comment

…pot runtime.

bridgekeeper · 2023-08-03T06:49:50Z

👋 Welcome back yzheng! 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-08-03T06:51:21Z

@mur47x111 The following labels will be automatically applied to this pull request:

graal
hotspot

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.

mlbridge · 2023-08-03T06:57:51Z

Webrevs

mur47x111 · 2023-08-03T06:52:45Z

src/hotspot/cpu/x86/vm_version_x86.cpp

    break;
+  case vmIntrinsics::_electronicCodeBook_encryptAESCrypt:
+  case vmIntrinsics::_electronicCodeBook_decryptAESCrypt:
+  case vmIntrinsics::_galoisCounterMode_AESCrypt:


Stubs for these intrinsics are generated with the following CPU features.
See

jdk/src/hotspot/cpu/x86/stubGenerator_x86_64_aes.cpp

Lines 159 to 166 in 53ca75b

if (VM_Version::supports_avx512_vaes() && VM_Version::supports_avx512vl() && VM_Version::supports_avx512dq() ) {

StubRoutines::_cipherBlockChaining_decryptAESCrypt = generate_cipherBlockChaining_decryptVectorAESCrypt();

StubRoutines::_electronicCodeBook_encryptAESCrypt = generate_electronicCodeBook_encryptAESCrypt();

StubRoutines::_electronicCodeBook_decryptAESCrypt = generate_electronicCodeBook_decryptAESCrypt();

StubRoutines::_galoisCounterMode_AESCrypt = generate_galoisCounterMode_AESCrypt();

} else {

StubRoutines::_cipherBlockChaining_decryptAESCrypt = generate_cipherBlockChaining_decryptAESCrypt_Parallel();

}

mur47x111 · 2023-08-03T06:54:10Z

src/hotspot/cpu/x86/vm_version_x86.cpp

+    }
+    break;
+  case vmIntrinsics::_bigIntegerRightShiftWorker:
+  case vmIntrinsics::_bigIntegerLeftShiftWorker:


See

jdk/src/hotspot/cpu/x86/stubGenerator_x86_64.cpp

Lines 4169 to 4172 in 53ca75b

if (VM_Version::supports_avx512_vbmi2()) {

StubRoutines::_bigIntegerRightShiftWorker = generate_bigIntegerRightShift();

StubRoutines::_bigIntegerLeftShiftWorker = generate_bigIntegerLeftShift();

}

mur47x111 · 2023-08-03T06:59:25Z

src/hotspot/cpu/x86/vm_version_x86.cpp

+    }
+    break;
+  case vmIntrinsics::_dcopySign:
+  case vmIntrinsics::_fcopySign:


See

jdk/src/hotspot/cpu/x86/x86.ad

Lines 1601 to 1609 in 53ca75b

case Op_CopySignD:

case Op_CopySignF:

if (UseAVX < 3 || !is_LP64) {

return false;

}

if (!VM_Version::supports_avx512vl()) {

return false;

}

break;

mur47x111 · 2023-08-03T06:59:51Z

src/hotspot/cpu/x86/vm_version_x86.cpp

+  case vmIntrinsics::_maxF_strict:
+  case vmIntrinsics::_minF_strict:
+  case vmIntrinsics::_maxD_strict:
+  case vmIntrinsics::_minD_strict:


See

jdk/src/hotspot/cpu/x86/x86.ad

Lines 1534 to 1543 in 53ca75b

#ifdef _LP64

case Op_MaxD:

case Op_MaxF:

case Op_MinD:

case Op_MinF:

if (UseAVX < 1) { // enabled for AVX only

return false;

}

break;

#endif

mur47x111 · 2023-08-03T07:07:02Z

src/hotspot/share/opto/c2compiler.cpp

+    break;
+  case vmIntrinsics::_electronicCodeBook_decryptAESCrypt:
+    if (StubRoutines::electronicCodeBook_decryptAESCrypt() == nullptr) return false;
+    break;


These two intrinsics were marked as supported on non-x86 platforms where the underlying stubs are not generated

mur47x111 · 2023-08-03T07:09:18Z

src/hotspot/cpu/x86/vm_version_x86.hpp

  static bool supports_avx256only()   { return (supports_avx2() && !supports_evex()); }
  static bool supports_avxonly()      { return ((supports_avx2() || supports_avx()) && !supports_evex()); }
  static bool supports_sha()          { return (_features & CPU_SHA) != 0; }
-  static bool supports_fma()          { return (_features & CPU_FMA) != 0 && supports_avx(); }


jdk/src/hotspot/cpu/x86/vm_version_x86.cpp

Lines 1154 to 1158 in 53ca75b

if (supports_fma() && UseSSE >= 2) { // Check UseSSE since FMA code uses SSE instructions

if (FLAG_IS_DEFAULT(UseFMA)) {

UseFMA = true;

}

} else if (UseFMA) {

implies fma intrinsic can be used without AVX

https://bugs.openjdk.org/browse/JDK-8181616 added support_avx() check because new Fma vectorization needs AVX: https://cr.openjdk.org/~vdeshpande/8181616/webrev.01/
Then we hit bug https://bugs.openjdk.org/browse/JDK-8182114 and bandaid it by restoring UseSSE check.
That change came before 8296168 which switch off UseAVX if UseSSE < 4:
https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/vm_version_x86.cpp#L908

This FMA check happens after UseSSE and UseAVX are set. I suggest to remove UseSSE check here instead and keep support_avx().

Saying that. You may remove support_avx() here but you need to add it to assembler vector instructions which have only support_fma() check now.

Thanks for the references! I have reverted this change and will adjust the Graal intrinsic accordingly.

mur47x111 · 2023-08-03T07:16:09Z

/label remove hotspot

mur47x111 · 2023-08-03T07:16:23Z

/label add hotspot-compiler

openjdk · 2023-08-03T07:16:52Z

@mur47x111
The hotspot label was successfully removed.

openjdk · 2023-08-03T07:17:28Z

@mur47x111
The hotspot-compiler label was successfully added.

dougxc

The JVMCI changes look good to me but someone else still needs to review the C1, C2 and shared assembler changes.

openjdk · 2023-08-03T07:42:41Z

@mur47x111 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:

8313372: [JVMCI] Export vmIntrinsics::is_intrinsic_available results to JVMCI compilers.

Reviewed-by: dnsimon, kvn

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 109 new commits pushed to the master branch:

049b55f: 8314019: Add gc logging to jdk/jfr/event/gc/detailed/TestZAllocationStallEvent.java
a39ed10: 8314116: C2: assert(false) failed: malformed control flow after JDK-8305636
1de5bf1: 8314106: C2: assert(is_valid()) failed: must be valid after JDK-8305636
5c91622: 8314117: RISC-V: Incorrect VMReg encoding in RISCV64Frame.java
6bbcef5: 8313948: Remove unnecessary static fields defaultUpper/defaultLower in sun.net.PortConfig
b88c273: 8313743: Make fields final in sun.nio.ch
ec0cc63: 8313904: [macos] All signing tests which verifies unsigned app images are failing
7332502: 8314139: TEST_BUG: runtime/os/THPsInThreadStackPreventionTest.java could fail on machine with large number of cores
8f1c134: 8313798: [aarch64] sun/tools/jhsdb/HeapDumpTestWithActiveProcess.java sometimes times out on aarch64
1232677: 8313244: NM flags handling in configure process
... and 99 more: https://git.openjdk.org/jdk/compare/cff25dd574203d0840d11ce083a5b825fb26d61d...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 (@dougxc, @vnkozlov) 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).

dean-long · 2023-08-09T08:46:36Z

I don't having the same logic in two places, because then those two places need to be kept in sync. Either the stubs should be generated based on is_intrinsic_supported(), or is_intrinsic_supported() should check if the stub was generated.

mur47x111 · 2023-08-10T14:28:39Z

I don't having the same logic in two places, because then those two places need to be kept in sync. Either the stubs should be generated based on is_intrinsic_supported(), or is_intrinsic_supported() should check if the stub was generated.

I have dropped the redundant CPU feature checks, and for those intrinsics with stubs, I tested if the stub pointer is nullptr in C2Compiler::is_intrinsic_supported

vnkozlov

Good.

vnkozlov · 2023-08-12T00:33:30Z

@mur47x111 please, rerun testing with latest version before integration.

mur47x111 · 2023-08-14T06:04:33Z

Passed tier1-3

mur47x111 · 2023-08-14T06:05:40Z

/integrate

openjdk · 2023-08-14T06:06:00Z

@mur47x111
Your change (at version 028aaf5) is now ready to be sponsored by a Committer.

dougxc · 2023-08-14T08:54:04Z

/sponsor

openjdk · 2023-08-14T08:56:20Z

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

049b55f: 8314019: Add gc logging to jdk/jfr/event/gc/detailed/TestZAllocationStallEvent.java
a39ed10: 8314116: C2: assert(false) failed: malformed control flow after JDK-8305636
1de5bf1: 8314106: C2: assert(is_valid()) failed: must be valid after JDK-8305636
5c91622: 8314117: RISC-V: Incorrect VMReg encoding in RISCV64Frame.java
6bbcef5: 8313948: Remove unnecessary static fields defaultUpper/defaultLower in sun.net.PortConfig
b88c273: 8313743: Make fields final in sun.nio.ch
ec0cc63: 8313904: [macos] All signing tests which verifies unsigned app images are failing
7332502: 8314139: TEST_BUG: runtime/os/THPsInThreadStackPreventionTest.java could fail on machine with large number of cores
8f1c134: 8313798: [aarch64] sun/tools/jhsdb/HeapDumpTestWithActiveProcess.java sometimes times out on aarch64
1232677: 8313244: NM flags handling in configure process
... and 99 more: https://git.openjdk.org/jdk/compare/cff25dd574203d0840d11ce083a5b825fb26d61d...master

Your commit was automatically rebased without conflicts.

openjdk · 2023-08-14T08:56:32Z

@dougxc @mur47x111 Pushed as commit 4164693.

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

Allow jvmci compiler to query if an intrinsic is disabled by the HotS…

7328edc

…pot runtime.

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

openjdk bot added graal graal-dev@openjdk.org hotspot hotspot-dev@openjdk.org labels Aug 3, 2023

update is_intrinsic_supported for _dcopySign,_fcopySign.

fba2316

mur47x111 commented Aug 3, 2023

View reviewed changes

openjdk bot removed the hotspot hotspot-dev@openjdk.org label Aug 3, 2023

openjdk bot added the hotspot-compiler hotspot-compiler-dev@openjdk.org label Aug 3, 2023

dougxc approved these changes Aug 3, 2023

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Aug 3, 2023

avoid duplicating cpu feature checks.

c6e6dc6

mur47x111 added 2 commits August 10, 2023 16:30

cleanup.

1d2b83e

revert change in supports_fma.

028aaf5

vnkozlov approved these changes Aug 12, 2023

View reviewed changes

openjdk bot added the sponsor Pull request is ready to be sponsored label Aug 14, 2023

dougxc approved these changes Aug 14, 2023

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Aug 14, 2023

openjdk bot closed this Aug 14, 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 Aug 14, 2023

	if (VM_Version::supports_avx512_vaes() && VM_Version::supports_avx512vl() && VM_Version::supports_avx512dq() ) {
	StubRoutines::_cipherBlockChaining_decryptAESCrypt = generate_cipherBlockChaining_decryptVectorAESCrypt();
	StubRoutines::_electronicCodeBook_encryptAESCrypt = generate_electronicCodeBook_encryptAESCrypt();
	StubRoutines::_electronicCodeBook_decryptAESCrypt = generate_electronicCodeBook_decryptAESCrypt();
	StubRoutines::_galoisCounterMode_AESCrypt = generate_galoisCounterMode_AESCrypt();
	} else {
	StubRoutines::_cipherBlockChaining_decryptAESCrypt = generate_cipherBlockChaining_decryptAESCrypt_Parallel();
	}

	if (VM_Version::supports_avx512_vbmi2()) {
	StubRoutines::_bigIntegerRightShiftWorker = generate_bigIntegerRightShift();
	StubRoutines::_bigIntegerLeftShiftWorker = generate_bigIntegerLeftShift();
	}

	case Op_CopySignD:
	case Op_CopySignF:
	if (UseAVX < 3 \|\| !is_LP64) {
	return false;
	}
	if (!VM_Version::supports_avx512vl()) {
	return false;
	}
	break;

	#ifdef _LP64
	case Op_MaxD:
	case Op_MaxF:
	case Op_MinD:
	case Op_MinF:
	if (UseAVX < 1) { // enabled for AVX only
	return false;
	}
	break;
	#endif

	if (supports_fma() && UseSSE >= 2) { // Check UseSSE since FMA code uses SSE instructions
	if (FLAG_IS_DEFAULT(UseFMA)) {
	UseFMA = true;
	}
	} else if (UseFMA) {

8313372: [JVMCI] Export vmIntrinsics::is_intrinsic_available results to JVMCI compilers. #15133

8313372: [JVMCI] Export vmIntrinsics::is_intrinsic_available results to JVMCI compilers. #15133

Uh oh!

Conversation

mur47x111 commented Aug 3, 2023 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Webrev

Uh oh!

bridgekeeper bot commented Aug 3, 2023

Uh oh!

openjdk bot commented Aug 3, 2023

Uh oh!

mlbridge bot commented Aug 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mur47x111 commented Aug 3, 2023

Uh oh!

mur47x111 commented Aug 3, 2023

Uh oh!

openjdk bot commented Aug 3, 2023

Uh oh!

openjdk bot commented Aug 3, 2023

Uh oh!

dougxc left a comment

Choose a reason for hiding this comment

Uh oh!

openjdk bot commented Aug 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dean-long commented Aug 9, 2023

Uh oh!

mur47x111 commented Aug 10, 2023

Uh oh!

vnkozlov left a comment

Choose a reason for hiding this comment

Uh oh!

vnkozlov commented Aug 12, 2023

Uh oh!

mur47x111 commented Aug 14, 2023

Uh oh!

mur47x111 commented Aug 14, 2023

Uh oh!

openjdk bot commented Aug 14, 2023

Uh oh!

dougxc commented Aug 14, 2023

Uh oh!

openjdk bot commented Aug 14, 2023

Uh oh!

openjdk bot commented Aug 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

mur47x111 commented Aug 3, 2023 •

edited by openjdk bot

Loading

mlbridge bot commented Aug 3, 2023 •

edited

Loading

openjdk bot commented Aug 3, 2023 •

edited

Loading