RFR: 8244523: Shenandoah: Remove null-handling in LRB expansion

Roman Kennke rkennke at redhat.com
Wed May 6 14:58:52 UTC 2020


>> See discussion in:
>> https://bugs.openjdk.java.net/browse/JDK-8244523
> 
>> It also removes the ideal-handling for CmpP. This attempted to eliminate
>> barriers when doing if (obj == null), but this is already dealt-with in
>> is_redundant(), and it also adds a case to is_redundant() to eliminate
>> LRB when the only consumer is the uncommon-trap of a NULL-check (in the
>> CallStaticJava case). Both are needed to avoid accidentally keeping an
>> explicit null-check for what should become an implicit null-check instead.
> 
> This feels like something not really related to this removal? No sense to split it out?

It is related. The CmpP and uncommon-trap-handling are there
specifically to deal with null-checking. If I leave it out, we'll see
extra null-checks being generated and regressions in benchmarks.

>> Webrev:
>> http://cr.openjdk.java.net/~rkennke/JDK-8244523/webrev.00/
> 
> Mostly cosmetics:
> 
> *) Comment indents:
> 
>  486   fields[TypeFunc::Parms+0] = value_type; // original field value
>  487   fields[TypeFunc::Parms+1] = TypeRawPtr::BOTTOM;   // original load address
> 
> *) This needs a comment, something like "uncommon traps do not need barriers, values would be
> handled during deopt" or some such.
> 
> 2932         if (n->as_CallStaticJava()->uncommon_trap_request() != 0) {
> 2933           break;
> 2934         }

Ok:
Diff:
http://cr.openjdk.java.net/~rkennke/JDK-8244523/webrev.01.diff/
Full:
http://cr.openjdk.java.net/~rkennke/JDK-8244523/webrev.01/

Good?

Roman



More information about the shenandoah-dev mailing list