Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8255522: [lworld] References to biased pattern remain in markWord::is_unlocked() #245

Closed
wants to merge 3 commits into from

Conversation

MrSimms
Copy link
Member

@MrSimms MrSimms commented Oct 28, 2020

  • more gtest mark word tests
  • assert any use of "biased locking" (there are some existing "has_biased_pattern()" asserts not guarded "UseBiasedLocking")
  • remove biased_lock_mask_in_place from is_unlocked

Progress

  • Change must not contain extraneous whitespace

Issue

  • JDK-8255522: [lworld] References to biased pattern remain in markWord::is_unlocked()

Reviewers

Download

$ git fetch https://git.openjdk.java.net/valhalla pull/245/head:pull/245
$ git checkout pull/245

@bridgekeeper
Copy link

bridgekeeper bot commented Oct 28, 2020

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

@openjdk
Copy link

openjdk bot commented Oct 28, 2020

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

8255522: [lworld] References to biased pattern remain in markWord::is_unlocked()

Reviewed-by: fparain, skuksenko

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 275 new commits pushed to the lworld branch:

  • e9724e5: 8256400: [lworld] C2 compilation fails with assert(addp->is_AddP() && addp->outcnt() > 0) failed: Don't process dead nodes
  • dfa9e45: 8256391: [lworld] C2 compilation fails with assert(EnableValhalla) failed: should only be used for inline types
  • 1c3bb00: 8256329: [lworld] C2 compilation fails with assert(!t->is_flat() && !t->is_not_flat()) failed: Should have been optimized out
  • 74e40a5: Merge jdk
  • bfa060f: 8256051: nmethod_entry_barrier stub miscalculates xmm spill size on x86_32
  • 96e0261: 8256106: Bypass intrinsic/barrier when calling Reference.get() from Finalizer
  • 3c3469b: 8256020: Shenandoah: Don't resurrect objects during evacuation on AS_NO_KEEPALIVE
  • 2e19026: 8253064: monitor list simplifications and getting rid of TSM
  • 421a7c3: 8256182: Update qemu-debootstrap cross-compilation recipe
  • 6247736: 8256018: Adler32/CRC32/CRC32C missing reachabilityFence
  • ... and 265 more: https://git.openjdk.java.net/valhalla/compare/a15b665f9550947a8478bd65af33855af3915e86...lworld

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 lworld branch, type /integrate in a new comment.

@mlbridge
Copy link

mlbridge bot commented Oct 28, 2020

Webrevs

@MrSimms MrSimms marked this pull request as draft Oct 28, 2020
@openjdk openjdk bot removed the rfr label Oct 28, 2020
@MrSimms MrSimms marked this pull request as ready for review Oct 28, 2020
@openjdk openjdk bot added the rfr label Oct 28, 2020
@MrSimms MrSimms requested review from fparain and kuksenko Nov 2, 2020
fparain
fparain approved these changes Nov 2, 2020
Copy link
Collaborator

@fparain fparain left a comment

Guarding around the bias pattern seems good.

I'm wondering if is_neutral() could benefit from a comment that both the 2 lock bits and the inline type bit are tested in a single operation (unlocked_value is usually used on the two last bits, but in this case, it is used on the last three bits).

There're still a lot of calls to has_bias_pattern() in the zero interpreter, but I'm not sure would should take care of that.

Fred

Copy link
Collaborator

@kuksenko kuksenko left a comment

It looks good for me.
With very minor notes.
Now biased_lock_pattern and inline_type_pattern are equals.The comment that EnableValhalla and UseBiasedLocking can't be set to true both is located higher in the markWord.hpp text. Maybe it's make sense to repeat it near that constants? To make it more clear.

How frequently "markWord::print_on" is used? Should we add printing "inline type" markword info?

@MrSimms MrSimms closed this Nov 9, 2020
@MrSimms MrSimms deleted the 8255522 branch Nov 9, 2020
@MrSimms MrSimms restored the 8255522 branch Nov 9, 2020
@MrSimms MrSimms reopened this Nov 9, 2020
@MrSimms
Copy link
Member Author

MrSimms commented Nov 17, 2020

/integrate

@openjdk openjdk bot closed this Nov 17, 2020
@openjdk openjdk bot added integrated and removed ready rfr labels Nov 17, 2020
@openjdk
Copy link

openjdk bot commented Nov 17, 2020

@MrSimms Since your change was applied there have been 275 commits pushed to the lworld branch:

  • e9724e5: 8256400: [lworld] C2 compilation fails with assert(addp->is_AddP() && addp->outcnt() > 0) failed: Don't process dead nodes
  • dfa9e45: 8256391: [lworld] C2 compilation fails with assert(EnableValhalla) failed: should only be used for inline types
  • 1c3bb00: 8256329: [lworld] C2 compilation fails with assert(!t->is_flat() && !t->is_not_flat()) failed: Should have been optimized out
  • 74e40a5: Merge jdk
  • bfa060f: 8256051: nmethod_entry_barrier stub miscalculates xmm spill size on x86_32
  • 96e0261: 8256106: Bypass intrinsic/barrier when calling Reference.get() from Finalizer
  • 3c3469b: 8256020: Shenandoah: Don't resurrect objects during evacuation on AS_NO_KEEPALIVE
  • 2e19026: 8253064: monitor list simplifications and getting rid of TSM
  • 421a7c3: 8256182: Update qemu-debootstrap cross-compilation recipe
  • 6247736: 8256018: Adler32/CRC32/CRC32C missing reachabilityFence
  • ... and 265 more: https://git.openjdk.java.net/valhalla/compare/a15b665f9550947a8478bd65af33855af3915e86...lworld

Your commit was automatically rebased without conflicts.

Pushed as commit a3fb148.

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

@MrSimms MrSimms deleted the 8255522 branch Nov 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants