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