[PATCH] D93376: [LangRef] Clarify the semantics of lifetime intrinsics

Thu Dec 17 12:31:50 PST 2020

jdoerfert added a comment.

In D93376#2461235 <https://reviews.llvm.org/D93376#2461235>, @nlopes wrote:

> In D93376#2461218 <https://reviews.llvm.org/D93376#2461218>, @jdoerfert wrote:
>
>> What is the reason to restrict it to allocas? Just that we don't emit it right now? I don't see how that makes it conceptually better.
>
> Potentially we could use these intrinsics with heap allocations such that after inlining they could maybe help to remove mallocs & friends. And maybe help alias analysis (Though the way basicAA works doesn't make implementing this any easy).
> One of the reasons why it is restricted to allocas ATM is that the existence of `lifetime.start(%p)` determines that `%p = alloca` doesn't really allocate space right away. So we need to be able to syntactically determine all inputs to lifetime.start intrinsics. Which is harder if you start supporting non-alloca'ed objects, pointer arithmetic and everything else.

IMHO, this is backwards. We don't need to restrict the usage to X in order to make it easier to use. We need to check at the use site if it is X and only proceed then.

> I suspect CodeGen would break right now if you use these intrinsics with non-alloca ptrs.

I'd argue this is a CodeGen bug.

> To summarize, I think we should restrict these intrinsics to alloca'ed ptrs for now as LLVM doesn't support anything else. If in the future someone extends them to support other kind of objects, then the semantics can be updated.

I'm still missing the why beyond: "Because some passes don't want to check `isa<AllocaInst>(V->stripPointerCasts())` before using the lifetime marker".

>> This change looks to me like it tries to fix a set of issues and at the same time do something else, that is restrict the functionality based on some use case.
>
> Not at all. It's just documenting the implicit assumptions all over LLVM's code, from IR to CodeGen. It's not changing anything. That's why we called out some CodeGen folks to ensure what's written reflects the assumptions of the stack coloring algorithm.

It absolutely is changing something. The lang ref right now is not excluding the use of this for heap memory. You can argue a use for heap memory is wrong and breaks hidden assumptions but the more straight forward explanation is that the users should verify if their assumptions hold, especially if it is trivial to do so. As I said before, there are two parts because some of the clarifications addressed here are just that, clarifications. Without them the intrinsics don't make much sense and we should certainly adopt them. However, heap memory and partial object lifetimes make sense on their own and both can be checked easily.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93376/new/

https://reviews.llvm.org/D93376