RFR 8216327 [lworld][c1] checkcast for value type should throw NullPointerException with null operand

[Tobias Hartmann]

Hi Ioi,

this looks good, thanks for the refactoring!

You don't need to pass "/* deoptimize */ false" because false is the default (no new webrev required).

Best regards,
Tobias


On 08.01.19 23:28, Ioi Lam wrote:
> Hi Tobias,
> 
> Thanks for the review. The reason I was deoptimizing was I didn't know how to distinguish between
> the Point.box vs Point.val case, so I let the interpreter decide whether to throw or not.
> 
> Anyway, I've fixed it so that the C1 code can properly decide whether to throw NPE, so there's no
> need to deoptimize.
> 
> I also renamed get_never_null() to is_klass_never_null(), so it's more apparent what this function
> does.
> 
> What do you think?
> 
> http://cr.openjdk.java.net/~iklam/valhalla/8216327-c1-checkcast-null.v02/
> 
> 
> On 1/8/19 8:29 AM, Tobias Hartmann wrote:
>> Hi Ioi,
>>
>> what about Point.box? No null check is needed in that case.
>>
>> Also, why do you need to deoptimize?
>>
>> Best regards,
>> Tobias
>>
>> On 08.01.19 06:29, Ioi Lam wrote:
>>> https://bugs.openjdk.java.net/browse/JDK-8216327
>>> http://cr.openjdk.java.net/~iklam/valhalla/8216327-c1-checkcast-null.v01/
>>>
>>> Please review this small fix.
>>>
>>> When we have something like this:
>>>
>>> static Point test() {
>>>      Object o = null;
>>>      Point p = (Point)o;
>>>      return p;
>>> }
>>>
>>> Deoptimize and let the interpreter generate the NPE.
>>>
>>> Thanks
>>> - Ioi
>>>
>>>
>>>
>>>
>>>
>>>