> 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 ------------- Changes: - all: https://git.openjdk.org/valhalla/pull/2197/files - new: https://git.openjdk.org/valhalla/pull/2197/files/48ca3cf7..0592eb94 Webrevs: - full: https://webrevs.openjdk.org/?repo=valhalla&pr=2197&range=01 - incr: https://webrevs.openjdk.org/?repo=valhalla&pr=2197&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/valhalla/pull/2197.diff Fetch: git fetch https://git.openjdk.org/valhalla.git pull/2197/head:pull/2197 PR: https://git.openjdk.org/valhalla/pull/2197
