[lworld] Initial C1 support for value types

Ioi Lam ioi.lam at oracle.com
Wed Nov 21 01:18:33 UTC 2018


I've added a FIXME to the comments regarding is_flattened(), and removed 
the "only load those fields" comments in putfield/getfield. I've pushed 
the changes.

Thanks
- Ioi

On 11/20/18 12:26 PM, Frederic Parain wrote:
>
>> On Nov 20, 2018, at 04:13, Tobias Hartmann <tobias.hartmann at oracle.com> wrote:
>>
>> Hi Ioi,
>>
>> thanks for sending this for review!
>>
>> Two minor things I've noticed in c1_GraphBuilder.cpp:
>> - "is_flattened() can return true for fields that are not value types". This should not happen and I
>> haven't seen this with C2 (which also uses the CI). We should add a "TODO" here to look at this later.
> This is a behavior I’ve observed while tracing execution during a bug investigation.
> Analyzing the ciField code, there’re some paths where the constructor returns before
> initializing the _is_flattened field. The fix is quite obvious but I wrote the c1_GraphBuilder
> code before I had time to fully analyzed the ciField code.
>
>> - I don't understand this comment "// Only load those fields who are not modified". I think it only
>> makes sense in the withfield context but not for putfield/getfield, right?
> Right, the copy/past needed some clean up that have not been done :-(
>
> Fred
>
>> No need for another webrev, please go ahead and push this.
>>
>> Thanks,
>> Tobias
>>
>> On 19.11.18 05:01, Ioi Lam wrote:
>>> Hi,
>>>
>>> Here's a webrev for initial C1 support. Most of the work is done by Fred. I added support for
>>> aaload/aastore.
>>>
>>> http://cr.openjdk.java.net/~iklam/valhalla/c1-initial-checkin.v01/
>>>
>>> C1 is disabled by default. To enable it, specify -XX:+EnableValhallaC1. When this flag is specified,
>>> C2 is disabled.
>>>
>>> To run the regression tests, use this:
>>>
>>>     cd test/hotspot/jtreg/compiler/valhalla/valuetypes
>>>     jtreg -Dtest.c1=true TestBasicFunctionality.java
>>>
>>> Currently only tests derived from ValueTypeTest are supported this way. Other tests won't work,
>>> especially if they try to parse C2 output.
>>>
>>> TestBasicFunctionality can run a bunch of test cases but will fail at test27().
>>>
>>> My plan is to work on acmp, and then gradually make all the test cases pass with
>>> TestBasicFunctionality.
>>>
>>> Thanks
>>>
>>> - Ioi
>>>
>>>
>>>
>>>
>>>



More information about the valhalla-dev mailing list