8299730: Add trivial call linker option

[JornVernee]

Add a trivial call linker option. This option acts as a hint to the runtime which indicates that a downcall is extremely short live (comparable to calling an empty function), and does not call back into Java.

Some implementations (like ours) might use this information to apply additional optimizations that are only applicable to trivial functions (dropping thread state transitions).

Note that the VM code diff looks kind of messy, but I essentially just put 3 if statements around the code that deals with the thread state transitions before and after the call. This also required moving the declarations of the Labels around though (so they are accessible from both ifs on the return path), which is probably messing up the diff view.

---

Progress

• Change must not contain extraneous whitespace
• Change must be properly reviewed (1 review required, with at least 1 Committer)

Issue

• JDK-8299730: Add trivial call linker option

Reviewers

• Maurizio Cimadamore (@mcimadamore - Committer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/panama-foreign pull/771/head:pull/771
$ git checkout pull/771

Update a local copy of the PR:
$ git checkout pull/771
$ git pull https://git.openjdk.org/panama-foreign pull/771/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 771

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/panama-foreign/pull/771.diff

[bridgekeeper]

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

[mlbridge]

Webrevs

• 04: Full - Incremental (8a1136f7)
• 03: Full - Incremental (b8d17c99)
• 02: Full - Incremental (21e945c1)
• 01: Full - Incremental (f397cf20)
• 00: Full (cc0ea3ca)

[openjdk]

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

8299730: Add trivial call linker option

Reviewed-by: mcimadamore

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 118 new commits pushed to the foreign-memaccess+abi branch:

• add0cf9: 8301228: Add more ways to resize zero-length memory segments
• a2822b6: Automatic merge of master into foreign-memaccess+abi
• 4e71fa3: Automatic merge of jdk:master into master
• d7aa87f: 8300176: URLEncoder/URLDecoder static fields should be private static final
• 107e184: 8301179: Replace NULL with nullptr in share/gc/serial/
• b77abc6: 8301178: Replace NULL with nullptr in share/gc/epsilon/
• f7da09c: 8301164: Remove unused ResourceStack class
• 6e4710b: 8300253: Introduce AArch64 nzcv accessors
• c6b3f2d: 8301129: Link to debuginfo files should only be made after stripping
• 938b409: 8301133: IGV: NPE occurs when creating a diff graph with a graph in a different folder
• ... and 108 more: https://git.openjdk.org/panama-foreign/compare/219d0ab43019b9efd4b109c8a441b86321ac2c80...foreign-memaccess+abi

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.

➡️ To integrate this PR with the above commit message to the foreign-memaccess+abi branch, type /integrate in a new comment.

[JornVernee]

Forgot to add the benchmarks before. Added them now.

Some numbers on my machine:

Benchmark                                  Mode  Cnt  Score   Error  Units
CallOverheadConstant.jni_blank             avgt   30  4.271 � 0.012  ns/op
CallOverheadConstant.panama_blank          avgt   30  3.300 � 0.022  ns/op
CallOverheadConstant.panama_blank_trivial  avgt   30  2.853 � 0.010  ns/op
CallOverheadVirtual.jni_blank              avgt   30  4.278 � 0.017  ns/op
CallOverheadVirtual.panama_blank           avgt   30  3.665 � 0.014  ns/op
CallOverheadVirtual.panama_blank_trivial   avgt   30  3.254 � 0.011  ns/op

The difference seems to be very minimal on my machine, but others have reported more significant improvements. This could still be improved by intrinsifying in C2 maybe, but not sure by how much, and if that's worth the complexity.

[mcimadamore]

Forgot to add the benchmarks before. Added them now.

Some numbers on my machine:

Benchmark                                  Mode  Cnt  Score   Error  Units
CallOverheadConstant.jni_blank             avgt   30  4.271 � 0.012  ns/op
CallOverheadConstant.panama_blank          avgt   30  3.300 � 0.022  ns/op
CallOverheadConstant.panama_blank_trivial  avgt   30  2.853 � 0.010  ns/op
CallOverheadVirtual.jni_blank              avgt   30  4.278 � 0.017  ns/op
CallOverheadVirtual.panama_blank           avgt   30  3.665 � 0.014  ns/op
CallOverheadVirtual.panama_blank_trivial   avgt   30  3.254 � 0.011  ns/op

The difference seems to be very minimal on my machine, but others have reported more significant improvements. This could still be improved by intrinsifying in C2 maybe, but not sure by how much, and if that's worth the complexity.

Nice!

[JornVernee]

The new test that tests that we can't do an upcall when a function is trivial was failing on the zero VM. zero just ignores the linker option, so it is also impossible to detect. I don't see any merit in trying to make sure that an upcall fails on zero though when the linker option is otherwise ignored. So, I've just disabled the test on zero instead.

[mcimadamore]

The new test that tests that we can't do an upcall when a function is trivial was failing on the zero VM. zero just ignores the linker option, so it is also impossible to detect. I don't see any merit in trying to make sure that an upcall fails on zero though when the linker option is otherwise ignored. So, I've just disabled the test on zero instead.

I agree - trivial should be best effort - perhaps something worth a note in the javadoc (e.g. that some linker implementation might reserve a right to refuse? Or even throw UOE when creating the unsupported option?)

[JornVernee]

Note that the current javadoc already hints that the implementation may (or may not) apply additional optimizations:

         * Using this linker option is a hint which some implementations may use to apply
         * optimizations that are only valid for trivial functions.

I don't think throwing a UOE is necessary in this case. (Though in general I think a particular linker implementation refusing a linker option is something that we want to allow, if only just in case)

[JornVernee]

/integrate

[openjdk]

Going to push as commit 200a116.
Since your change was applied there have been 118 commits pushed to the foreign-memaccess+abi branch:

• add0cf9: 8301228: Add more ways to resize zero-length memory segments
• a2822b6: Automatic merge of master into foreign-memaccess+abi
• 4e71fa3: Automatic merge of jdk:master into master
• d7aa87f: 8300176: URLEncoder/URLDecoder static fields should be private static final
• 107e184: 8301179: Replace NULL with nullptr in share/gc/serial/
• b77abc6: 8301178: Replace NULL with nullptr in share/gc/epsilon/
• f7da09c: 8301164: Remove unused ResourceStack class
• 6e4710b: 8300253: Introduce AArch64 nzcv accessors
• c6b3f2d: 8301129: Link to debuginfo files should only be made after stripping
• 938b409: 8301133: IGV: NPE occurs when creating a diff graph with a graph in a different folder
• ... and 108 more: https://git.openjdk.org/panama-foreign/compare/219d0ab43019b9efd4b109c8a441b86321ac2c80...foreign-memaccess+abi

Your commit was automatically rebased without conflicts.

[openjdk]

@JornVernee Pushed as commit 200a116.

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