[LLVMdev] RFC: implicit null checks in llvm

[Sanjoy Das]

> 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?

> Let’s just be clear that it is not recommended for the frontend to produce these intrinsics. They are a compiler backend convenience. (I don’t want InstCombine or any other standard pass to start trafficking in these.)

Agreed.

> This is a platform dependent intrinsic. There’s nothing wrong with a platform specific size for the unmapped page zero if we don’t already have one.

Agreed.

> Good. This is a codegen feature. We can’t say that enough. If you really cared about the best codegen this would be done in machine IR after scheduling and target LoadStore opts.

I thought about doing this very late, but I figured that pattern
matching on control flow will be much easier while we are still in
LLVM IR.

> 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.

> 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.

-- Sanjoy