> Please review the following patch which re-enables virtual thread tests 
> `TestVirtualThreads.java`‎ and `Fuzz.java`.
> 
> First, this patch fixes the AArch64 virtual thread code to adapt to the 
> changes in [JDK-8367151](https://bugs.openjdk.org/browse/JDK-8367151) where 
> the valid FP is now stored in copy `#1` instead of copy `#2`:
> 
> https://github.com/openjdk/valhalla/blob/c0b679480a10bc9c71aa75ed26b3cfd0e69d294c/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp#L5960-L6007
> 
> With that change we can now simplify the `stackChunk` walking code and the 
> thawing patching logic, since we don't need to keep track of two separate 
> stack pointers, one to access the saved pc (`_unextended_sp[-1]`) and another 
> to access the saved fp (`_sp[-2]`). Also, method `compiled_frame_details()` 
> enables simplification of `FreezeBase::sender`.
> Follow-up changes from 
> [JDK-8371993](https://bugs.openjdk.org/browse/JDK-8371993) uncovered that in 
> `ContinuationHelper::Frame::return_pc_address` we were reading the saved pc 
> from the wrong location. This is also fixed in this PR. Going forward though, 
> it seems the issue is with `f.real_fp()` which should include the extra added 
> size for extended frames.
> 
> Second, the x64 code has been updated so that the layout of the extended 
> frames is the same as with AArch64, and that we also only use the `#1` 
> copies. This not only aligns behavior across both platforms but also allows 
> simplification of the virtual thread code as mentioned above with AArch64. 
> Changes in [JDK-8372806](https://bugs.openjdk.org/browse/JDK-8372806) 
> uncovered the same issue with `ContinuationHelper::Frame::return_pc_address` 
> and that is fixed as well.
> Note: While working on this, I noticed that for extended frames, with 
> `-XX:+PreserveFramePointer`, rbp is set to the location of copy `#2` rather 
> than copy `#1`. Storing bad values in these locations will break the chain of 
> pointers though, so we probably want to set rbp to location of copy `#1`. 
> Same with AArch64.
> 
> Finally, for both AArch64 and x64 I updated `frame::describe_pd` to show 
> correct locations of saved return pc and FP for extended frames. I still kept 
> the locations where `#2` copies are stored. This has proven useful for 
> debugging purposes.
> 
> Changes were tested by running both `TestVirtualThreads.java‎` and 
> `Fuzz.java`, job `valhalla-comp-stress` in mach5, as well as tiers1-3. I also 
> run `TestVirtualThreads.java‎` with extra flags, including `-Xcomp 
> -XX:+SafepointALot -XX:+DeoptimizeALot` as reported in [JDK-8370177](https...

Patricio Chilano Mateo has updated the pull request with a new target base due 
to a merge or a rebase. The pull request now contains three commits:

 - Review comments
 - Merge branch 'lworld' into JDK-8377714
 - v1

-------------

Changes: https://git.openjdk.org/valhalla/pull/2085/files
  Webrev: https://webrevs.openjdk.org/?repo=valhalla&pr=2085&range=01
  Stats: 205 lines in 21 files changed: 62 ins; 74 del; 69 mod
  Patch: https://git.openjdk.org/valhalla/pull/2085.diff
  Fetch: git fetch https://git.openjdk.org/valhalla.git pull/2085/head:pull/2085

PR: https://git.openjdk.org/valhalla/pull/2085

Reply via email to