[PATCH] D82821: [WebAssembly] Added 64-bit memory.grow/size/init/copy/fill

[Wouter van Oortmerssen via Phabricator via llvm-commits]

aardappel marked an inline comment as done.
aardappel added inline comments.


================
Comment at: llvm/include/llvm/IR/IntrinsicsWebAssembly.td:216
+            [llvm_i32_ty, llvm_i32_ty, llvm_ptr_ty, llvm_anyint_ty,
+             llvm_anyint_ty],
             [IntrWriteMem, IntrInaccessibleMemOrArgMemOnly, WriteOnly<ArgIndex<2>>,
----------------
dschuff wrote:
> tlively wrote:
> > dschuff wrote:
> > > tlively wrote:
> > > > I'm surprised this doesn't break the bulk-memory-intrinsics.ll test since this makes the intrinsic polymorphic. It would be good to add the 64-bit versions of these intrinsics to that test as well, and if the polymorphism starts causing problems, it would probably be a good idea to replace the second `llvm_anyint_ty` here with a `LLVMMatchType<3>` so you only have to specify a single type in the intrinsic name.
> > > > 
> > > > Alternatively, you could just delete these intrinsics and the corresponding clang builtins. They are extremely useless because there is no way to know what segment ID to pass before linking. I'm surprised we don't have intrinsics and builtins for memory.copy and memory.fill, which could actually be useful.
> > > > 
> > > > Another test you might want to update is the bulk-memory-encodings.s MC test.
> > > do these segment IDs get rewritten by the linker, i.e. could they be symbolic?  Are they only synthesized by the linker now or do we generate them in the backend? (I forget and I think we went back and forth on that).  Would it make sense to have e.g. in the assembly language but not in C/LLVM IR?
> > No, they cannot currently be symbolic. Right now they have to be constant integers. Even if they could be symbolic, we would have to change the linker to give symbols referenced that way their own segments, rather than merging them with all the other symbols into one big segment. If we made those changes, I could see these instructions being very useful in low-level assembly programs, though. The Binaryen segment optimizations are already robust enough that we wouldn't have to do any work in Binaryen for this feature.
> ok thanks. seems worth keeping in mind, but obviously not a high priority unless we get a use case or request.
Your suggestion of `LLVMMatchType<3>` gets me `error GA8ED74C0: Parameter #4 has out of bounds matching number 3`, and looking at the other definitions, it seems these refer to the previous list, not the current one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82821