RFR: 8297036: Generalize C2 stub mechanism

Roman Kennke rkennke at openjdk.org
Tue Nov 29 12:10:27 UTC 2022


On Mon, 28 Nov 2022 09:50:54 GMT, Xiaolin Zheng <xlinzheng at openjdk.org> wrote:

> Hi Roman,
> 
> I felt there was something still vague to me, so I took another look into this issue earlier today and found another interesting thing.
> 
> It seems there are two issues reflected by this PR, but of course, this PR is only doing refactoring work... awesome.
> 
> The other issue is, it appears to me that [1] and [2] both lack a `cb->stubs()->maybe_expand_to_ensure_remaining();` before the `align()`s. After adding the expansion logic before the two places, failures are gone (on RISC-V).
> 
> So, in summary, there are two issues here (certainly, not related to this PR - this PR just interestingly triggers and lets us spot them):
> 
> 1. `output()->_stub_list._stubs` is always a `0` value.
> 2. the missing `cb->stubs()->maybe_expand_to_ensure_remaining();` before `align()` in the shared trampoline logic, as above-mentioned.
> 
> It appears to me that we already have got the expansion logic for the two stubs [3], and the size is `2048` - enough big value to cover the sizes of the stubs.
> 
> I would like to humbly suggest some solutions to it:
> 
> 1. A quick fix is to remove the `C2CodeStubList::measure_code_size()` for it always returns a `0` now (sorry for saying this), or I guess we can use some other approaches to calculate the correct node counts of the two kinds of stubs.
> 2. I guess I might need to file another PR to solve the missing expansion logic in shared trampolines.
> 
> I would like to hear what you think.
> 
> Best, Xiaolin
> 
> [1]
> 
> https://github.com/openjdk/jdk/blob/43d1173605128126dda0dc39ffc376b84065cc65/src/hotspot/cpu/aarch64/codeBuffer_aarch64.cpp#L55
> 
> 
> [2]
> https://github.com/openjdk/jdk/blob/43d1173605128126dda0dc39ffc376b84065cc65/src/hotspot/cpu/riscv/codeBuffer_riscv.cpp#L56
> 
> 
> [3] https://github.com/openjdk/jdk/pull/11188/files#diff-96c31ff7167c1300458cf557427ee89af5250035ecbc2f189817c793a328a502R74

I think I understand now. You are right - when code size is 'measured' we don't have any stubs, yet. That is because the stubs only get generated while all the other assembly code is emitted, i.e. after code buffers are generated. This problem is pre-existing and C2SafepointPollStub got that part wrong before.

However, we *do* call maybe_expand_to_ensure_remaining() before align(), that happens in C2CodeStubList::emit() before each stub gets emitted. I changed that part now to try expansion only with the amount of code that each stub requires instead of some maximum size. I'm also checking that each stub generates as much code as it reports it would. I am not sure how useful that is, tbh. But it helps to implement the size() methods (which you need to do now in RISCV). Start with implementing them to return 0, do a build, and change the value to what the check reports.

The only other way to improve the situation is if we would first emit the whole method into the code buffer, and then measure and create a new buffer only for the stubs. It would be very small, and I don't know if it's worth the effort or if that is possible at all. WDYT?
Please let me know if that fixes your problem!

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

PR: https://git.openjdk.org/jdk/pull/11188


More information about the hotspot-compiler-dev mailing list