RFR(S) JDK-8206140 [lworld] Move return value null checks into the callee

[Ioi Lam]

On 7/10/18 5:01 AM, Tobias Hartmann wrote:
> Hi Ioi,
>
> thanks for looking at this!
>
> On 10.07.2018 07:07, Ioi Lam wrote:
>> http://cr.openjdk.java.net/~iklam/valhalla/8206140_lworld_null_check_in_callee.v02/
Hi Tobias,

Thanks for the comments. I've updated a new version:

http://cr.openjdk.java.net/~iklam/valhalla/8206140_lworld_null_check_in_callee.v03/

> - doCall.cpp: The cast to NOTNULL is only correct if we know that the return value is a value type
> (which can never be null). Currently, you are always casting to NOTNULL which is not correct.

Fixed. Now it looks like:

  670             const Type*       sig_type = 
TypeOopPtr::make_from_klass(ctype->as_klass());
  671             if (ct == T_VALUETYPE) {
  672               // A NULL ValueType cannot be returned to compiled 
code. The 'areturn' bytecode
  673               // handler will deoptimize its caller if it is about 
to return a NULL ValueType.
  674               sig_type = sig_type->join_speculative(TypePtr::NOTNULL);
  675             }
> - TestLWorld.java: Where is GetNullAsm and RarelyUsedValueUserAsm defined?
>
I added them to the webrev.
>> To improve interpreter speed (so we won't excessively trap into the VM whenever a
>> null is returned -- this is especially important for methods that are NOT returning
>> a VT), I added 2 bits in Method::_flags. These allow the interpreter to quickly
>> check if the areturn is working on a method that returns a VT.
>>
>> The flags handling is complicated by the fact that "legacy" classes may return a VT
>> (see [1]). Please see my comments around Method::check_returning_vt. I added a new
>> test case for this -- see TestLWorld::test78). Anyway, I am not happy with this check,
>> so if you can think of a better way, please let me know.
> Great that you were able to create a test for this. I don't think we can easily avoid that check but
> I think it's sufficient if you add the detailed explanation to the test and remove the else branch
> (or add a short comment) in method.cpp.

I think it's better to keep the comments in the code, as the test case 
may get moved or become out of sync with the code. I've added a comment 
in the test case to point back to the explanation in method.cpp.

Thanks
- Ioi
> Someone more familiar with the interpreter/runtime should look at this as well.
>
> Thanks,
> Tobias
>
>