8332032: C2: Remove ExpandSubTypeCheckAtParseTime flag

[chhagedorn]

With this patch I propose to remove the diagnostic product flag ExpandSubTypeCheckAtParseTime for the following reasons:

• Expanding sub type checks eagerly during parse time has a maintenance cost. We've had to make special fixes due to skipping SubTypeCheckNodes in the past (recent example: JDK-8328702, where the idea of removing this flag was first discussed).
• This stress option has not helped much to find bugs. Going through JBS, maybe only 1 or 2 bugs can be attributed to this flag over the last 4 years - and even for those, it could have very well be that the flag was not required because it was often accompanied by other stress flags such as StressReflecitiveCode.
• We currently have a bug in Valhalla (JDK-8331912) which only happens with ExpandSubTYpeCheckAtParseTime. The reason is that we lose flatness information due to the eager sub type expansion. Later, data becomes top and the corresponding (already expanded) sub type check fails to fold control as well, leading to a broken graph. The simplest solution is to remove ExpandSubTYpeCheckAtParseTime.

Thanks,
Christian

---

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-8332032: C2: Remove ExpandSubTypeCheckAtParseTime flag (Enhancement - P4)

Reviewers

• Tobias Hartmann (@TobiHartmann - Reviewer)
• Roberto Castañeda Lozano (@robcasloz - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 19430

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

Using diff file

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

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back chagedorn! 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]

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

8332032: C2: Remove ExpandSubTypeCheckAtParseTime flag

Reviewed-by: thartmann, rcastanedalo

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

• 6cda4c5: 8321543: Update NSS to version 3.96
• c003c12: 8331865: Consolidate size and alignment checks in LayoutPath
• 6d718ae: 8324341: Remove redundant preprocessor #if's checks
• 9b64ece: 8332904: ubsan ppc64le: c1_LIRGenerator_ppc.cpp:581:21: runtime error: signed integer overflow: 9223372036854775807 + 1 cannot be represented in type 'long int'
• 3d4eb15: 8302744: Refactor Hotspot container detection code
• 2cca83b: 8332880: JFR GCHelper class recognizes "Archive" regions as valid
• b8ae11e: 8332960: ubsan: classListParser.hpp:159:12: runtime error: load of value 2101478704, which is not a valid value for type 'ParseMode'
• 9a83dfe: 8332431: NullPointerException in JTable of SwingSet2
• 01060ad: 8325083: jdk/incubator/vector/Double512VectorTests.java crashes in Assembler::vex_prefix_and_encode
• 673f767: 8285506: Unify os::vsnprintf implementations
• ... and 13 more: https://git.openjdk.org/jdk/compare/2edb6d98133d8bd6dc4527c7497c460283fdc53e...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.

[openjdk]

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

• hotspot-compiler

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]

Webrevs

• 00: Full (01d05db2)

[TobiHartmann]

Looks good to me.

[chhagedorn]

Thanks Tobias for your review!

[robcasloz]

Looks good!

[chhagedorn]

Thanks Roberto for your review!

[chhagedorn]

/integrate

[openjdk]

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

• ba323b5: 8333047: Remove arena-size-workaround in jvmtiUtils.cpp
• 22ef827: 8320396: Class-File API ClassModel::verify should include checks from hotspot/share/classfile/classFileParser.cpp
• 2ab8ab5: 8332858: References with escapes have broken positions after they are transformed
• 1b7d59f: 8333303: Issues with DottedVersion class
• e304a8a: 8333307: Don't suppress jpackage logging in tests when it is detecting packaging tools in the system
• 3634a91: 8332751: Broken link in VirtualMachine.html
• ffb0867: 8331485: Odd Results when Parsing Scientific Notation with Large Exponent
• 79f4998: 8321314: Reinstate disabling the compiler's default active annotation processing
• ec88c6a: 8332917: failure_handler should execute gdb "info threads" command on linux
• b3e29db: 8333108: Update vmTestbase/nsk/share/DebugeeProcess.java to don't use finalization
• ... and 48 more: https://git.openjdk.org/jdk/compare/2edb6d98133d8bd6dc4527c7497c460283fdc53e...master

Your commit was automatically rebased without conflicts.

[openjdk]

@chhagedorn Pushed as commit 95c8a69.

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