implements llvm intrinsics and clang intrinsics for
memory.init and data.drop.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
- Build Status
Buildable 28039 Build 28038: arc lint + arc unit
Event Timeline
clang/include/clang/Basic/BuiltinsWebAssembly.def | ||
---|---|---|
29 |
| |
30 | The same thing..
| |
clang/lib/CodeGen/CGBuiltin.cpp | ||
13483 | Not sure if we can use llvm_unreachable here, because we can certainly reach here if a user uses this builtin with a non-const variable. In this file people often just used assert for user errors, which is not ideal either. I haven't used it myself, but looking at the code, the recommended way to signal an error looks like to use [[ https://github.com/llvm/llvm-project/blob/db7fbcb038f095622a3e6847ecd6ff80bdc2483a/clang/lib/CodeGen/CodeGenFunction.h#L2092-L2094 | CodeGenFunction::ErrorUnsupported ]] function, as in here. We used llvm_unreachable for SIMD builtins too, but maybe we can fix it later. | |
13494 | Do we need to pass types here to make intrinsic names overloaded like llvm.wasm.memory.init.i32.i32.i8, unless this intrinsic also support operands of other types, which it doesn't? The same for data.drop builtin. | |
llvm/include/llvm/IR/IntrinsicsWebAssembly.td | ||
119 |
| |
121 |
| |
122 | I told you we needed this, but on second thought, because we don't really use MachineMemOperand, maybe we don't need it..? :$ The standard memcpy/memmove/memset intrinsics don't have it either. So if the code runs ok without these I think we can delete it. The same for data.drop intrinsic. Sorry for the incorrect info and confusion. | |
125 | The same, llvm_i32_ty |
- Address comments
- Use Ui in builtin signatures
- Remove unnecessary intrinsic polymorphism
- Tweak intrinsic properties
clang/include/clang/Basic/BuiltinsWebAssembly.def | ||
---|---|---|
29 | We use Ii instead of i when the argument needs to be a compile time constant because it is encoded as a literal in the corresponding instruction. I was imagining that we could add a new builtin once multiple memories are available, but perhaps it would be better to add an argument to the builtin now and error out if it is not zero. I will do that. Yes, Ui seems reasonable. | |
30 |
| |
clang/lib/CodeGen/CGBuiltin.cpp | ||
13483 | llvm_unreachable is appropriate here because non-constant expressions will have been caught earlier by the type checking. A follow-up PR updating SIMD intrinsics to use Ui and appropriate error handling sounds good. | |
13494 | Fixed. I had been copying llvm.memcpy, but I agree it is better to not be polymorphic. | |
llvm/include/llvm/IR/IntrinsicsWebAssembly.td | ||
121 |
| |
122 | No problem! |
clang/lib/CodeGen/CGBuiltin.cpp | ||
---|---|---|
13483 |
Oh, does Ii ensures that? That's cool... |