On Mon, 9 Mar 2026 09:18:21 GMT, Marc Chevalier <[email protected]> wrote:
>> The IR framework rarely complains that `TestLWorld::test178` >> >> https://github.com/openjdk/valhalla/blob/a6b20a25e2b045f675b202c74f391cf70f865cd0/test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestLWorld.java#L5086-L5091 >> >> is not compilable. After investigation, it turns out it's because the >> register allocator fails here >> >> https://github.com/openjdk/valhalla/blob/a6b20a25e2b045f675b202c74f391cf70f865cd0/src/hotspot/share/opto/chaitin.cpp#L569-L577 >> >> I've looked at the graph, and got a 854M IGV dump... I should have used a >> smaller log level! But the thing is that this graph is very complicated. >> That's because `Value178` has many (transitive) fields (more than 150, if I >> counted well, including null markers). Expanding the `acmp` turns into a lot >> of nodes, with a very intricate structure that makes the register allocation >> rarely fail. >> >> If we raise the limit of `27` in the code above, the test doesn't fail >> anymore. But is it a common problem? I've changed the bailout with an >> assert, and I've played with the limit, all on tiers 1-3, prechecking and >> (valhalla-)stress. With the current limit of 27 I got 2 failures on mainline >> and valhalla. For a limit of 20, I got a single but different failure on >> mainline, and 8 on valhalla. With a limit of 15, we have about 60 failures >> on mainline, and (curiously) only about 20 on Valhalla. >> >> I've done a bit more experiments, with less precise counting, and my general >> feeling is that the limit is generous for most cases, but there are a few >> outliers that needs a quite high limit, and our `acmp` expansion seems to >> create some in some cases. We also need to emphasize that the `Value178` >> here is quite huge, it's probably not your typical value class. >> >> At this point, the pragmatic thing to do is to simply allow the compilation >> of this method to fail. It seems that the RA is not wrong that this problem >> is hard. It doesn't seem reasonable to mess with the RA to make avoid such >> rare failures. I've filed >> [JDK-8378943](https://bugs.openjdk.org/browse/JDK-8378943) to make sure we >> don't accidentally accept unexpected bailout reasons. >> >> Thanks, >> Marc > > Marc Chevalier has updated the pull request incrementally with one additional > commit since the last revision: > > Add TODO That looks reasonable and I agree with your analysis! ------------- Marked as reviewed by chagedorn (Committer). PR Review: https://git.openjdk.org/valhalla/pull/2197#pullrequestreview-3914045693
