[LLVMdev] RFC: implicit null checks in llvm

[Andrew Trick]

> On Apr 23, 2015, at 1:29 PM, Sanjoy Das <sanjoy at playingwithpointers.com> wrote:
> 
>> The intrinsics need to be lowered to a pseudo instruction just like patchpoint (so that a stackmap can be emitted). In my mind the real issue here is how to teaching this pseudo instruction to emit the proper load/store for the target.
> 
> Do you a specific set of issues that you think could be problematic?

One problem is that we’re bypassing instruction selection. You’ll probably need to encode the load width as an instruction operand. You might only want to handle integer loads.

>> I don’t think making it profile-guided is important. Program behavior can change after compilation and you’re back to the same problem. I think recovering from repeated traps is important. That’s why you need to combine this feature with either code invalidation points or patching implemented via llvm.stackmap, patchpoint, (or statepoint) — they’re all the same thing.
> 
> Without profiling information, how will the frontend communicate to
> the backend that an explicit null check "never" fails and hence may be
> replaced with an implicit null check (or that it has been observed to
> fail, and should not be replaced with an implicit null check)?  In the
> scheme I'm thinking of, a null check is made implicit only if the (branch)
> probability of it failing is approximately zero.

It’s better to have PGO. I’m saying that it is neither necessary nor sufficient. You can speculate that check does not fail and recover from repeated failures by patching or recompiling. What you absolutely need is runtime profiling of failed checks. But my argument is irrelevant from LLVM side. I think the decision point is whether the pass should require BlockFrequency, and I agree that it should.

>> Well, you need the features that patchpoint gives you (stackmaps entries) and you’ll need to use patchpoints or stackmaps anyway for invalidation or patching. So why are you bothering with a totally new, independent intrinsic? Why not just extend the existing intrinsics. We could have a variant that
>> 
>> - emits a load instead of a call
>> 
>> - looks at the landing pad to generate a special stackmap entry in addition to the normal exception table (I don’t even see why you need this, except that the runtime doesn’t know how to parse an exception table.)
> 
> This sounds good -- reusing the patchpoint intrinsics will save me a
> lot of work.

Yeah, the goal is to reuse the infrastructure. It could still be a separate intrinsic with shared implementation if that turns out to be easier. I’m unconvinced either way. An interesting question is how you will handle repeated traps: by invalidating the whole compiled method or just patching in an explicit branch. I suspect you’ll invalidate the compiled method, in which case a separate intrinsic is fine. To do the later you would obviously need all the features of a patchpoint.

Andy