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

[Thomas Lively via Phabricator via llvm-commits]

tlively 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>>,
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82821