8283740: C1: Convert flag TwoOperandLIRForm to a constant on all platforms

[dafedafe]

The TwoOperandLIRForm flag (LIR requires src1 and dst to match in binary LIR ops) can be set at runtime but each platform defines a default value that is the only one accepted. Although some platform could theoretically support both, there is no realistic scenario for this.
The flag is removed in favour of a constant. This is defined as enum and its assigned value is defined in the platform-specific def.

---

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-8283740: C1: Convert flag TwoOperandLIRForm to a constant on all platforms

Reviewers

• Tobias Hartmann (@TobiHartmann - Reviewer)
• Christian Hagedorn (@chhagedorn - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 11671

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

Using diff file

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

[bridgekeeper]

Hi @dafedafe, welcome to this OpenJDK project and thanks for contributing!

We do not recognize you as Contributor and need to ensure you have signed the Oracle Contributor Agreement (OCA). If you have not signed the OCA, please follow the instructions. Please fill in your GitHub username in the "Username" field of the application. Once you have signed the OCA, please let us know by writing /signed in a comment in this pull request.

If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please use "Add GitHub user dafedafe" as summary for the issue.

If you are contributing this work on behalf of your employer and your employer has signed the OCA, please let us know by writing /covered in a comment in this pull request.

[openjdk]

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

• hotspot
• shenandoah

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.

[TobiHartmann]

That looks good to me but I think all these platform dependent constants should be converted to static const declarations at some point. From what I understand, enums are used here only for historical reasons. Could you file a follow-up RFE to investigate?

[chhagedorn]

That looks good to me!

That looks good to me but I think all these platform dependent constants should be converted to static const declarations at some point. From what I understand, enums are used here only for historical reasons. Could you file a follow-up RFE to investigate?

That's a good idea to clean this up as well at some point. But probably too much for this PR. So, I agree to file a separate RFE for it.

[dafedafe]

/label remove shenandoah

[openjdk]

@dafedafe
The shenandoah label was successfully removed.

[TobiHartmann]

And FTR:

Due to bugs in certain (very old) compilers, there is widespread use of enums and avoidance of in-class initialization of static integral constant members. Compilers having such bugs are no longer supported. Except where an enum is semantically appropriate, new code should use integral constants.

https://github.com/openjdk/jdk/blob/master/doc/hotspot-style.md

[openjdk]

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

8283740: C1: Convert flag TwoOperandLIRForm to a constant on all platforms

Reviewed-by: thartmann, chagedorn

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

• 1622563: 8265688: Unused ciMethodType::ptype_at should be removed
• 3637660: 8298813: [C2] Converting double to float cause a loss of precision and resulting crypto.aes scores fluctuate
• 5e678f7: 8298824: C2 crash: assert(is_Bool()) failed: invalid node class: ConI
• ba942c2: 8298244: AArch64: Optimize vector implementation of AddReduction for floating point
• 7938f8c: 8298639: Perform I/O operations in bulk for RandomAccessFile
• bfa921a: 8160404: RelocationHolder constructors have bugs
• bf9a8ce: 8249826: 5 javax/net/ssl/SSLEngine tests use @ignore w/o bug-id
• 0eeaeb8: 8298808: Check script code on detecting the base locales
• 81e23ab: 8298809: Clean up vm/compiler/InterfaceCalls JMH
• 3696711: Merge
• ... and 60 more: https://git.openjdk.org/jdk/compare/220781fa56a9c8d3b64c5c6578ffd76b9edb795c...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 (@TobiHartmann, @chhagedorn) 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).

[mlbridge]

Webrevs

• 00: Full (cdfd2b12)

[dafedafe]

@TobiHartmann @chhagedorn thanks for the review!

[dafedafe]

/integrate

[openjdk]

@dafedafe
Your change (at version cdfd2b1) is now ready to be sponsored by a Committer.

[TobiHartmann]

/sponsor

[openjdk]

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

• ec95991: 8298736: Revisit usages of log10 in compiler code
• 1622563: 8265688: Unused ciMethodType::ptype_at should be removed
• 3637660: 8298813: [C2] Converting double to float cause a loss of precision and resulting crypto.aes scores fluctuate
• 5e678f7: 8298824: C2 crash: assert(is_Bool()) failed: invalid node class: ConI
• ba942c2: 8298244: AArch64: Optimize vector implementation of AddReduction for floating point
• 7938f8c: 8298639: Perform I/O operations in bulk for RandomAccessFile
• bfa921a: 8160404: RelocationHolder constructors have bugs
• bf9a8ce: 8249826: 5 javax/net/ssl/SSLEngine tests use @ignore w/o bug-id
• 0eeaeb8: 8298808: Check script code on detecting the base locales
• 81e23ab: 8298809: Clean up vm/compiler/InterfaceCalls JMH
• ... and 61 more: https://git.openjdk.org/jdk/compare/220781fa56a9c8d3b64c5c6578ffd76b9edb795c...master

Your commit was automatically rebased without conflicts.

[openjdk]

@TobiHartmann @dafedafe Pushed as commit 86d588b.

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