LRB midpath code quality

Roman Kennke rkennke at redhat.com
Wed Mar 6 09:39:18 UTC 2019


>>>>> Well yes, an implicit null-check would be good. It would never fire,
>>>>> because if the value is null, it would blow up earlier in the cset
>>>>> check. Or am I missing something?
>>>>
>>>> I suppose PhaseCFG::implicit_null_check() doesn't recognize the barrier
>>>> load as a candidate for implicit null checks for some reason.
>>>
>>> Yes, maybe.
>>>
>>> But can you explain to me why it should be needed at all? If a
>>> dominating (implicit?) null-check before the in-cset-check should catch
>>> all nulls on that path already? Why does it matter?
>>
>> Also, I don't think we want to generate an NPE/SEGV on that path. We
>> want to skip a bunch of stuff if the value is NULL.
> 
> We might just back-branch to the access, no? And let NPE be handled there?
> 
> In here:
>   https://paste.fedoraproject.org/paste/nMZb19apbJq0uOB9CboUOA
> 
> This branch:
> 
> 0x00007f46f04b9272: test   %rax,%rax
> 0x00007f46f04b9275: je     0x00007f46f04b9294
> 
> ...can just go to where every other branch is going:
> 
> 0x00007f46f04b9247: movl   $0x2a,0x20(%rax)
>    ; implicit exception: dispatches to 0x00007f46f04b9294

The interesting question is this: we generate ideal code for
null-check+branch before in-cset-check. What happened to this? Is it
gone because C2 proved the value to be non-null? Why is the cloned
branch not gone? Because it cannot be proven non-null? I believe this
whole implicit NPE is nonsensical there: we don't want NPE or SEGV
there, we want to branch back in case of null. I am trying a fix.

It'd look like this then:
https://paste.fedoraproject.org/paste/NHnR-sJFj8sJkESY5KS2XQ

Roman



More information about the shenandoah-dev mailing list