Without this change llvm will generate invalid wasm object files.
Details
Diff Detail
- Repository
- rL LLVM
- Build Status
Buildable 20243 Build 20243: arc lint + arc unit
Event Timeline
My biggest concern here is that this pass shouldn't affect prototype-possessing functions, and that appears to be satisfied :-).
Otherwise LGTM
lib/Target/WebAssembly/WebAssemblyAddMissingPrototypes.cpp | ||
---|---|---|
75 | This code more-or-less functions as a kind of "assert" on clang's convention of using vararg-only functions for prototypeless C functions. If that changes, or if someone accidentally creates nonconforming IR (say, with a different frontend) then they'll end up here, so it's probably worth just putting a big comment here saying that's what we're doing. Fortunately the use of the attribute minimizes risk of accidentally having this IR, so I think the actual code is fine. | |
test/CodeGen/WebAssembly/add-prototypes.ll | ||
12 | can we add a test with a non-call use as well (e.g. storing a pointer to the function)? |
This code more-or-less functions as a kind of "assert" on clang's convention of using vararg-only functions for prototypeless C functions. If that changes, or if someone accidentally creates nonconforming IR (say, with a different frontend) then they'll end up here, so it's probably worth just putting a big comment here saying that's what we're doing. Fortunately the use of the attribute minimizes risk of accidentally having this IR, so I think the actual code is fine.