LRB midpath code quality

Roman Kennke rkennke at redhat.com
Wed Mar 6 10:04:50 UTC 2019


>> Ok, but why has it been removed?
>> 1. In case C2 has proven the value to be not-NULL, I suppose we can just
>> as well not null-check in the fwd ptr load?
> 
> The null check is after the barrier. We load an oop field, emit the
> barrier, null check the oop and then do something with the oop. So at
> the point where the barrier is c2 hasn't proven the oop to be not null
> and there needs to be a null check. What the current code does is find
> that there's a null check after the barrier, find the null check never
> fails, move it in the barrier so we have no need for an explicit test
> for null.
> 
>> 2. In case it has been turned into an implicit NULL-check I argue that
>> this is wrong: we don't want an NPE or SEGV there, we want to skip the
>> whole path. Also, wouldn't the annotated assembly say so like 'implicit
>> exception: dispatches to ...' ?
> 
> The null check is there because it was observed to never trigger. So in
> practice there will be no SEGV. And if it ever triggers then the code
> will deoptimize and be recompiled. On the recompilation, the null check
> won't be seen as unlikely by c2 and it won't be moved around anymore.
> 
>> Sure. But that would only ever throw an NPE? Or can it be made to branch
>> back too? And if so, it would have to be done *before* the cset check,
>> not after, otherwise it would go BOOM there.
> 
> It's not going to branch back, it's going to deoptimize, very rarely. So
> this is saving the need for a check for null and a branch and we don't
> give up anything in exchange. Why would you not want that?

ok, I understand now. Here's the patch with the null-check-cloning intact:
http://cr.openjdk.java.net/~rkennke/streamline-lrb-midpath/streamline-lrb-midpath2.patch

It still generates the explicit null-check inside the barrier midpath. 
Can you check how to eliminate that?

Thanks,
Roman


More information about the shenandoah-dev mailing list