[PATCH] D101140: [WebAssembly][CodeGen] Allow for externref/funcref local variables

Andy Wingo via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 29 01:55:00 PDT 2021


wingo added a comment.

In D101140#2724740 <https://reviews.llvm.org/D101140#2724740>, @tlively wrote:

> Nice! This turns out to be relatively simple. It would be good to add more tests with reference typed locals mixed in with normal locals, etc.

Thanks for the feedback!  Agreed with regards to tests obviously.  Following up directly as regards the most important question, "is it actually robust":

> Also, what happens when one of the alloca values is used by something other than a load or a store? Is it just an ISel failure at that point?

Let us assume that because of source restrictions, all allocas are static (in the sense of `AllocaInst::isStaticAlloca()`).  The alloca's are in a non-integral address space so the only operation you can do on them is to pass the pointer value around, or load from it, or store to it.  (If I missed a potential use, I am happy to be corrected.)    We handle the load and store cases.  Source restrictions prevent passing locals by-reference to other functions, so I think we are good there as well.  I have no idea what might happen if the source restrictions aren't respected though -- good thing the SVE people have done most of the work on the front-end already because getting a coherent story there would be complicated otherwise!

There is one "hole in the racket", as apparently we say in french -- externref/funcref locals which have no uses.  You wouldn't want these leaving isel as live stack objects in the MachineFrameInfo.  I am thinking it may be more robust to run a quick pre-pass through the stack objects after the static alloca's are collected in `FunctionLoweringInfo::set` to eagerly transform them to locals and mark them dead, even if there are no uses.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101140



More information about the llvm-commits mailing list