8255271: Avoid generating duplicate interpreter entries for subword types #816

cl4es · 2020-10-22T18:19:59Z

Top-of-stack optimizations are generally missing for byte, char, short and boolean, and for several types of entry points we already avoid generating entries and instead redirect to the int variant.

This patch removes generation of "specialized" variants for byte, char, short and boolean more thoroughly after verifying they all generate the same code as their int specialization. This slightly reduces overhead of generating the interpreter and the size thereof.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Testing

	Linux x32	Linux x64	Windows x64	macOS x64
Build	✔️ (1/1 passed)	✔️ (5/5 passed)	✔️ (2/2 passed)	✔️ (2/2 passed)
Test (tier1)		✔️ (9/9 passed)	❌ (1/9 failed)	✔️ (9/9 passed)

Failed test task

Windows x64 (hs/tier1 compiler)

Issue

JDK-8255271: Avoid generating duplicate interpreter entries for subword types

Reviewers

Ioi Lam (@iklam - Reviewer)
Coleen Phillimore (@coleenp - Reviewer)

Download

$ git fetch https://git.openjdk.java.net/jdk pull/816/head:pull/816
$ git checkout pull/816

bridgekeeper · 2020-10-22T18:21:52Z

👋 Welcome back redestad! 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 · 2020-10-22T18:24:53Z

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

hotspot

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 · 2020-10-22T20:07:16Z

Webrevs

00: Full (a92a31e)

iklam

LGTM

openjdk · 2020-10-22T20:50:58Z

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

8255271: Avoid generating duplicate interpreter entries for subword types

Reviewed-by: iklam, coleenp

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

12daf2b: 8255340: [s390] build failure after JDK-8255208
9007bc2: 8255129: [PPC64, s390] Check vector_size_supported and add VectorReinterpret node
df79257: 8255274: [PPC64, s390] wrong StringLatin1.indexOf version matched
55a0cad: 8254982: (tz) Upgrade time-zone data to tzdata2020c
ac1748e: 8254670: SVE test uses linux-specific api
0e92053: 8254286: Wrong inference in switch expression with "null" arm
63ce304: 8249675: x86: frequency extraction from cpu brand string is incomplete
e52156d: 8255067: Restore Copyright line in file modified by 8253191
c520469: 8255131: G1CollectedHeap::is_in() returns wrong result
107fb9c: 8255049: Remove support for the hsdis decode_instructions entry point in hotspot
... and 10 more: https://git.openjdk.java.net/jdk/compare/a0b687bfb279761b79d3fbad8f9dfae9b12c225e...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.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

coleenp

Looks good to me.

coleenp · 2020-10-23T13:08:37Z

src/hotspot/share/interpreter/templateInterpreter.cpp

+  _entry[btos] = ientry;
+  _entry[ztos] = ientry;
+  _entry[ctos] = ientry;
+  _entry[stos] = ientry;


next patch is to get rid of these entry points since I think nothing branches to them.

Thanks for reviewing.

I'll file an RFE to keep at this. I didn't want to keep pulling at this thread for this RFE since it seems like quite a chunk of code will be unravelled here.

cl4es · 2020-10-23T15:37:14Z

/integrate

openjdk · 2020-10-23T15:39:14Z

@cl4es Since your change was applied there have been 21 commits pushed to the master branch:

5ec1b80: 8255287: aarch64: fix SVE patterns for vector shift count
12daf2b: 8255340: [s390] build failure after JDK-8255208
9007bc2: 8255129: [PPC64, s390] Check vector_size_supported and add VectorReinterpret node
df79257: 8255274: [PPC64, s390] wrong StringLatin1.indexOf version matched
55a0cad: 8254982: (tz) Upgrade time-zone data to tzdata2020c
ac1748e: 8254670: SVE test uses linux-specific api
0e92053: 8254286: Wrong inference in switch expression with "null" arm
63ce304: 8249675: x86: frequency extraction from cpu brand string is incomplete
e52156d: 8255067: Restore Copyright line in file modified by 8253191
c520469: 8255131: G1CollectedHeap::is_in() returns wrong result
... and 11 more: https://git.openjdk.java.net/jdk/compare/a0b687bfb279761b79d3fbad8f9dfae9b12c225e...master

Your commit was automatically rebased without conflicts.

Pushed as commit cc86113.

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

cl4es added 10 commits October 2, 2020 00:43

Inline locator_address

cd1d9fa

Avoid generating TOS entries for subword types

fabcc53

Allow legitimate uses of pop/push(b-stos)

66cba2a

Coalesce more entry points

81bf96d

Add missing increment

576aba9

Gardening

202a380

typos in assert

60254a6

Merge branch 'master' into interpreter_init_opts

c6fde67

Introduce EntryPoint(ailfdv) and simplify

969fc2d

Revert accidental changes in templateInterpreterGenerator_x86

a92a31e

openjdk bot added the hotspot hotspot-dev@openjdk.org label Oct 22, 2020

cl4es marked this pull request as ready for review October 22, 2020 20:02

openjdk bot added the rfr Pull request is ready for review label Oct 22, 2020

iklam approved these changes Oct 22, 2020

View changes

openjdk bot added the ready Pull request is ready to be integrated label Oct 22, 2020

coleenp approved these changes Oct 23, 2020

View changes

openjdk bot closed this Oct 23, 2020

openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Oct 23, 2020

8255271: Avoid generating duplicate interpreter entries for subword types #816

8255271: Avoid generating duplicate interpreter entries for subword types #816

cl4es commented Oct 22, 2020 •

edited by openjdk bot

bridgekeeper bot commented Oct 22, 2020

openjdk bot commented Oct 22, 2020

mlbridge bot commented Oct 22, 2020

iklam left a comment

openjdk bot commented Oct 22, 2020 •

edited

coleenp left a comment

coleenp Oct 23, 2020

cl4es Oct 23, 2020

cl4es commented Oct 23, 2020

openjdk bot commented Oct 23, 2020

8255271: Avoid generating duplicate interpreter entries for subword types #816

8255271: Avoid generating duplicate interpreter entries for subword types #816

Conversation

cl4es commented Oct 22, 2020 • edited by openjdk bot

Progress

Testing

Issue

Reviewers

Download

bridgekeeper bot commented Oct 22, 2020

openjdk bot commented Oct 22, 2020

mlbridge bot commented Oct 22, 2020

Webrevs

iklam left a comment

Choose a reason for hiding this comment

openjdk bot commented Oct 22, 2020 • edited

coleenp left a comment

Choose a reason for hiding this comment

coleenp Oct 23, 2020

Choose a reason for hiding this comment

cl4es Oct 23, 2020

Choose a reason for hiding this comment

cl4es commented Oct 23, 2020

openjdk bot commented Oct 23, 2020

cl4es commented Oct 22, 2020 •

edited by openjdk bot

openjdk bot commented Oct 22, 2020 •

edited