8279225: [arm32] C1 longs comparison operation destroys argument registers

[snazarkin]

Several regression tests are failed on arm32 CPU if tiered compilation is enabled.

The list includes
java/math/BigDecimal/DivideMcTests
java/util/Arrays/Sorting.java
java/util/Arrays/SortingNearlySortedPrimitive.java
java/util/concurrent/tck/JSR166TestCase
java/util/stream/SliceOpTest.java
etc

It appears C1 comp_op for long operands destroys arguments registers:
void LIR_Assembler::comp_op(LIR_Condition condition, LIR_Opr opr1, LIR_Opr opr2, LIR_Op2* op) {
....
Register ylo = opr2->as_register_lo();
Register yhi = opr2->as_register_hi();
if (condition == lir_cond_equal || condition == lir_cond_notEqual) {
__ teq(xhi, yhi);
__ teq(xlo, ylo, eq);
} else {
__ subs(xlo, xlo, ylo); // <<< incorrect
__ sbcs(xhi, xhi, yhi); // <<< incorrect
}
...
}

Tested with hotspot_tier2, jdk_tier3 on linux_arm

---

Progress

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

Issue

• JDK-8279225: [arm32] C1 longs comparison operation destroys argument registers

Reviewers

• Hao Sun (@shqking - Author)
• Andrew Haley (@theRealAph - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/6934/head:pull/6934
$ git checkout pull/6934

Update a local copy of the PR:
$ git checkout pull/6934
$ git pull https://git.openjdk.java.net/jdk pull/6934/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 6934

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/6934.diff

[bridgekeeper]

👋 Welcome back snazarki! 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.

[snazarkin]

@bulasevich could you please check this fix?

[openjdk]

@snazarkin 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

• 01: Full - Incremental (3ee6f5c9)
• 00: Full (53916e49)

[bulasevich]

Good catch. The fix is Ok, thank you!

[shqking]

This fix looks good to me. (I'm not a Reviewer).

One thing to remind is that the PR and JBS should use the same title.

[shqking]

nit: we may want to use __ subs(Rtemp, xlo, ylo); here to align with the usage in match rules in arm.ad, e.g., compL_reg_reg_LEGT. It's okay to me if you use cmp anyway.

[theRealAph]

I agree.

[snazarkin]

Aligned with C2 implementation

[shqking]

Thanks for your update. LGTM. (I'm not a Reviewer)

[theRealAph]

OK.

[openjdk]

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

8279225: [arm32] C1 longs comparison operation destroys argument registers

Reviewed-by: haosun, aph

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

• 4f607f2: Merge
• 54b800d: 8271202: C1: assert(false) failed: live_in set of first block must be empty
• 2945b78: 8279195: Document the -XX:+NeverActAsServerClassMachine flag
• 6588bed: 8278889: AArch64: [vectorapi] VectorMaskLoadStoreTest.testMaskCast() test fail
• 7fea103: 8279134: Fix Amazon copyright in various files

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
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 (@theRealAph) 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).

[snazarkin]

/integrate

[openjdk]

@snazarkin
Your change (at version 3ee6f5c) is now ready to be sponsored by a Committer.

[alexeybakhtin]

/sponsor

[openjdk]

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

• 4f607f2: Merge
• 54b800d: 8271202: C1: assert(false) failed: live_in set of first block must be empty
• 2945b78: 8279195: Document the -XX:+NeverActAsServerClassMachine flag
• 6588bed: 8278889: AArch64: [vectorapi] VectorMaskLoadStoreTest.testMaskCast() test fail
• 7fea103: 8279134: Fix Amazon copyright in various files

Your commit was automatically rebased without conflicts.

[openjdk]

@alexeybakhtin @snazarkin Pushed as commit 299022d.

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