[PATCH] D108464: [clang][CodeGen] Refactor CreateTempAlloca function nest. NFC.

Tue Aug 24 01:36:19 PDT 2021

wingo added a subscriber: sbc100.
wingo added a comment.

Thanks again John & Thomas for your thoughts.

In D108464#2961595 <https://reviews.llvm.org/D108464#2961595>, @rjmccall wrote:

> In D108464#2960791 <https://reviews.llvm.org/D108464#2960791>, @tlively wrote:
>
>> I don't think it makes sense to present these concepts as "address spaces" to C/C++ users, but that's what we're using at the IR level.
>
> Yeah, at some point I'm willing to accept that this is your best option at the IR level, but I want to not jam this into the user-facing language unless it's really the right design approach.

I am absolutely not married to the particular approach in this patch series and am happy to explore the design space :)

In D108464#2960791 <https://reviews.llvm.org/D108464#2960791>, @tlively wrote:

> @wingo, are there cases where it is useful to declare variables as living in WebAssembly locals and not in the VM stack? I'm having trouble coming up with a case where leaving that up to the backend is not enough. We clearly need a way to prevent values from being written to main memory (AS 0), but it's not clear to me that we need a way to specifically allocate locals for them.

No, there are no cases that I know of.  From the IR & backend POV, the point as you say is to prevent values from being written to main memory.  It doesn't matter if they are in named locals or just temporaries.

The issue is that clang always lowers local variables as alloca's.  Clang needs to lower them to alloca's in AS 1 for reference types.  Without optimization, LLVM will lower an alloca in AS 1 to a WebAssembly local.  SSA conversion in SROA could lift it to an SSA variable which may avoid the local, in some cases.  So we are not specifically allocating a local for them, I agree that isn't the right way to express the requirement.

Also, being able to annotate non-reference-types with a `wasm_var` address space is not really part of the requirements.  It certainly has no utility for variables with automatic storage duration.  @sbc100 did mention that it could be useful for non-reference-typed globals, though, for ABI reasons.

In D108464#2961595 <https://reviews.llvm.org/D108464#2961595>, @rjmccall wrote:

> Right now this seems to be entirely type-directed: it's a special property of a couple of builtin types that you can't take the address of their objects and those objects can only live in specific places.  Having it be type-directed, but only to the point of saying that certain types *must* use certain address spaces, and then imposing all the other restrictions on those types as novel restrictions on those address spaces, feels like it's adding complexity to the language concept of address spaces without much benefit.

Yeah I could be getting this wrong here.

To expand a bit, it's a special property of a class of builtin types -- currently `externref` and `funcref` but future WebAssembly specifications <https://github.com/WebAssembly/gc/issues/81> will include reference types like `(struct i32 externref)`.  So there is a concept to factor out here that isn't specific to just `externref` and `funcref`.

@rjmccall do you think some kind of type attribute would make more sense?  If there is something already existing that I can take as a model, that would be helpful of course.

The utility we get from LangAS right now is (1) that it's a clear path to get alloca in AS 1 in codegen and (2) that it's a concept that Sema/ can query in order to detect errors and signal them to the user.  I am sure there are other ways to do this though!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108464