RFR: 8330159: [C2] Remove or clarify Compile::init_start

Yagmur Eren duke at openjdk.org
Thu Aug 29 09:47:18 UTC 2024


On Tue, 27 Aug 2024 07:36:21 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> Compile::init_start method contained only an assertion. To cleanup, this method is removed and the locations where this method is called are replaced with the corresponding assertion. See issue: [JDK-8330159](https://bugs.openjdk.org/browse/JDK-8330159)
>
> Hi @nelanbu, I don't think this is correct. In `Compile::start()`, we have the following code:
> https://github.com/openjdk/jdk/blob/b8e8e965e541881605f9dbcd4d9871d4952b9232/src/hotspot/share/opto/compile.cpp#L1121-L1131
> 
> It asserts that `failing()` is false. Therefore, `init_start()` bails out before checking the assert with `start()` which you now no longer do with your refactoring.
> 
> What you could do instead:
> - Simplify the code in `init_start()` to and add an assertion message:
> 
>   assert(failing() || s == start(), "should be StartNode");
> 
> - Change `init_start_node()` into a more meaningful name like `verify_start()`, as we are not actually initializing anything but rather sanity checking the start node.
> - Guard the method with `DEBUG_ONLY/ifdef ASSERT` since it's only calling an assert in debug VM and nothing in product VM.

Hello @chhagedorn, thanks a lot for your feedback! I updated accordingly. Hope it looks better now.

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

PR Comment: https://git.openjdk.org/jdk/pull/20715#issuecomment-2317179737


More information about the hotspot-compiler-dev mailing list