This covers both the existing memory functions as well as the new bulk memory proposal.
Added new test files since changes where also required in the inputs.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
@tlively I have no idea if my changes in IntrinsicsWebAssembly.td make sense, they merely pass the tests ;) please advise.
llvm/include/llvm/IR/IntrinsicsWebAssembly.td | ||
---|---|---|
216 | 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. |
llvm/include/llvm/IR/IntrinsicsWebAssembly.td | ||
---|---|---|
216 | 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? |
llvm/include/llvm/IR/IntrinsicsWebAssembly.td | ||
---|---|---|
216 | 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. |
llvm/include/llvm/IR/IntrinsicsWebAssembly.td | ||
---|---|---|
216 | ok thanks. seems worth keeping in mind, but obviously not a high priority unless we get a use case or request. |
llvm/include/llvm/IR/IntrinsicsWebAssembly.td | ||
---|---|---|
216 | 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? |
Removed unused init/drop intrinsics instead of trying to make them work for wasm64.
Also testing bulk-memory-encoding.s
llvm/test/MC/WebAssembly/bulk-memory-encodings.s | ||
---|---|---|
10 ↗ | (On Diff #275258) | Getting rid of the intrinsics and clang builtins is fine, but I think we should keep the instruction definitions and these tests around. They would still be useful for disassembling modules that contain these instructions in the linker-synthesized memory initialization function. |
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.