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

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

Commit messages:
 - Allow test178 to bailout

Changes: https://git.openjdk.org/valhalla/pull/2197/files
  Webrev: https://webrevs.openjdk.org/?repo=valhalla&pr=2197&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8375357
  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

Reply via email to