[lworld] RFR(M): Cleanup of null checks

[John Rose]

Reviewed.  This is really good; all those deleted lines are the
sign of a successful refactoring.

I have one suggestion:  After the refactoring make_from_oop now has
full responsibility for null checks.  The boolean parameter 'flattened'
now means "map null to default"; if it is false, then a null triggers deopt
which (I assume) eventually gets to an NPE.

My suggestion is to rename flattened to null2default or map_null_to_default
or default_on_null.  This will make the division of responsibility clearer between
make_from_oop and its users.  The general meaning of and policy concerning
"flattened" values is in flux, and a more specific name for this argument will make
sure it won't become incomprehensible in the future.

— John

On Jun 28, 2018, at 7:56 AM, Tobias Hartmann <tobias.hartmann at oracle.com> wrote:
> 
> Hi,
> 
> please review the following patch:
> http://cr.openjdk.java.net/~thartmann/valhalla/lworld/null_check_cleanup/
> 
> Changes:
> - Moved all null checks into ValueTypeNode::make_from_oop(). We are now emitting null checks if the
> oop can be null
> - Added asserts at the call sites to verify that we never emit null checks where it's not required
> - Fixed a problem with OSR where a null value type is accessed without a null check (see new test in
> TestOnStackReplacement.java)
> - Some refactoring and comment cleanups
> 
> All compiler and runtime tests pass.
> 
> Thanks,
> Tobias