RFR: 8322996: BoxLockNode creation fails with assert(reg < CHUNK_SIZE) failed: sanity [v5]

[Vladimir Kozlov]

On Thu, 25 Jan 2024 12:26:45 GMT, Daniel Lundén <dlunden at openjdk.org> wrote:

>> src/hotspot/share/opto/graphKit.cpp line 3473:
>> 
>>> 3471:   Node* box = _gvn.transform(new BoxLockNode(next_monitor()));
>>> 3472:   // Check for bailout after new BoxLockNode
>>> 3473:   if (failing()) { return nullptr; }
>> 
>> Do all callers of `shared_lock()` checks for `failing()` or returned `nullptr`?
>
> No, not the immediate callers at least. Below is a quick call graph analysis for the places where we create `BoxLockNode`s (up to the first bailout check). Should I add returns at all points in the call chain up to the first checks?
> 
> graphKit.cpp:3471 (this is in shared_lock)
>   locknode.cpp:196
>     parse2.cpp:2759
>       parse1.cpp:1594 (Checks for bailout at parse1.cpp:1595)
>   parse1.cpp:1264
>     parse1.cpp:582 (Checks for bailout at parse1.cpp:596)
> 
> parse1.cpp:227
>   parse1.cpp:579 (Checks for bailout at parse1.cpp:596)

I am not worry about exit in`load_interpreter_state()` there is check after call.

There is call to `record_profiled_parameters_for_speculation()` after `shared_lock()` call in `do_method_entry()`. It does not check `failing()`. May be add check `if (!failing())` when calling it (or inside it).

For `do_one_bytecode()` case the only question is if IGVN code can handle such bailout. It is called at the end of method. If not, we need to add the check.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17370#discussion_r1467186357