RFR: 8330159: [C2] Remove or clarify Compile::init_start
Christian Hagedorn
chagedorn at openjdk.org
Tue Aug 27 07:39:04 UTC 2024
On Mon, 26 Aug 2024 13:54:16 GMT, Yagmur Eren <duke 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.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/20715#issuecomment-2311782455
More information about the hotspot-compiler-dev
mailing list