RFR JDK-8207219: [lworld] C2 should not invoke a method if its signature has inconsistent use of ValueType

[Ioi Lam]

Hi Tobias,

I think your suggestion should work. I'll need to think about how it 
might work when inlining across different class loaders. This could be 
tricky when LPoint; is a ValueType in CL1, but not a ValueType in CL2. 
I'll write a bunch of test cases to cover the scenarios.

Thanks
- Ioi

On 7/16/18 1:44 AM, Tobias Hartmann wrote:
> Hi Ioi,
>
> Inlining of a method 'callee' with a value type in it's signature into a method 'caller' is now only
> possible if the class declaring 'caller' has the value type in it's attribute. I think that's too
> strong because there can be very long inlining chains where _task->method()->method_holder() is not
> aware of a value type being used deep inside that chain.
>
> I think we would need a check like this:
> If 'callee' has a (loaded) non-primitive type in its signature, check if that type is known to be a
> value type (Klass::is_value()) and if so throw an ICCE if the 'callee' holder does not declare it in
> its value type attribute. That way we guarantee that assumptions of the caller at compile time (for
> example, non-nullness of the returned value type) are consistent with assumptions of the callee.
>
> It would be good to add the test(s) to the webrev.
>
> Thanks,
> Tobias
>
> On 14.07.2018 22:14, Ioi Lam wrote:
>> http://cr.openjdk.java.net/~iklam/valhalla/8207219_c2_invoke_vt_consistency.v01/
>> https://bugs.openjdk.java.net/browse/JDK-8207219
>>
>> This patch prevents improper inlining: when you have programs like this
>>
>>      class VT { // knows that Point is a ValueType
>>          void crasher() {
>>              NonVT.foo();
>>          }
>>      }
>>
>>      class NonVT { // does NOT know that Point is a ValueType
>>          static Point x;
>>          static void foo() {
>>              x = bar(); // <-- cannot inline this call into VT.crasher
>>          }
>>
>>          static Point bar() {
>>              return null;
>>          }
>>      }
>>
>> If we inlined the bar() call into VT.crasher, it would break JDK-8206140 [1],
>> which assumes that a callee will never return a NULL value back to a
>> compiled caller.
>>
>> Thanks
>> - Ioi
>>
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-8206140
>>      [lworld] Move return value null checks into the callee
>>
>>