andykaylor wrote: I have a number of things to say here.
First, @E00N777, I apologize for throwing you into such a complicated situation. I thought the initial implementation would be less controversial than this, but to be honest this is the nature of compiler development so in that sense this is a good way to get familiar with the LLVM development processes. Second, I think we're conflating two things in the discussion above. The `cir.alloca` operation seems to be doing two things that are closely related but conceptually very different. (1) It allocates stack memory for a variable. (2) It declares a local variable. These are semantically very different. The actual `alloca` portion of this can and should be hoisted to the function entry in most cases. Because it is semantically moveable, it cannot be relied on to determine the lifetime of the variable. If we're going to try to deduce the lifetime of a variable from its position in CIR scopes, we really need a separate operation to declare the variable. Note that the `HoistAllocas` pass is currently being run before the `FlattenCFG` pass, and even if it weren't there is no reason that someone couldn't create a custom pipeline and run it as early as they like. I imagine that by now (and probably from the start of this discussion) @erichkeane is thinking that CIR has no business doing something as low-level as allocating stack memory, and I wouldn't disagree with that. Ideally, we'd treat locals much more like we treat globals, with a `cir.local` operation and a `cir.get_local` to get its address, and we wouldn't attempt the stack allocation until much later in the pipeline. One of my bigger concerns is that while we have a clear point in the pipeline after which CIR is required to be flat, we don't have any firm requirement that it be structured at any other point. We probably should, and I know we've been trying to generate the code that way (though it gets a little messy with goto and switch). In principle there is nothing stopping me from generating code that has a bunch of regions but also has a bunch of blocks with branches jumping between them. If we wanted to introduce a `cir.local` operation and rely on explicit scopes to determine its lifetime, we'd need to limit it to living in the first block of a region. Finally, I want to advocate for a purely practical approach to this. We will need to introduce lifetime markers at some point because LLVM IR uses them. We have a clear example from classic codegen for when and where they can be created to get them in the right place. I suggest that it makes sense to start with an implementation based closely on the classic codegen placement of lifetime markers to establish an initial correct state. Then, after we have that working, we can work towards a higher-level representation as discussed above, and the tests that we create during the initial low-level implementation will serve as guard-rails to keep the implementation correct (that is, we need to be able to lower to the same final result either way). tl;dr -- I think it makes sense to pursue the direction started by this PR. https://github.com/llvm/llvm-project/pull/199599 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
