Page MenuHomePhabricator

[WebAssembly] Add pass to infer prototypes for prototype-less functions
ClosedPublic

Authored by sbc100 on Jun 21 2018, 7:25 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

sbc100 created this revision.Jun 21 2018, 7:25 PM
sbc100 edited the summary of this revision. (Show Details)Jun 21 2018, 7:26 PM
sunfish accepted this revision.Jun 25 2018, 8:36 AM

My biggest concern here is that this pass shouldn't affect prototype-possessing functions, and that appears to be satisfied :-).

This revision is now accepted and ready to land.Jun 25 2018, 8:36 AM
sbc100 updated this revision to Diff 152751.Jun 25 2018, 12:50 PM
  • fix typo
  • cleanup
dschuff accepted this revision.Jul 2 2018, 2:06 PM

Otherwise LGTM

lib/Target/WebAssembly/WebAssemblyAddMissingPrototypes.cpp
74 ↗(On Diff #152751)

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
11 ↗(On Diff #152751)

can we add a test with a non-call use as well (e.g. storing a pointer to the function)?

sbc100 updated this revision to Diff 154926.Jul 10 2018, 9:05 PM
  • add to test
sbc100 updated this revision to Diff 154928.Jul 10 2018, 9:29 PM
sbc100 marked an inline comment as done.
  • add to test
  • add comment
This revision was automatically updated to reflect the committed changes.