This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Make sret parameter work with AddMissingPrototypes
ClosedPublic

Authored by aheejin on Jul 8 2019, 3:12 AM.

Details

Summary

Even with functions with no-prototype attribute, there can be an
argument sret (structure return) attribute, which is an optimization
when a function return type is a struct. Fixes PR42420.

Diff Detail

Repository
rL LLVM

Event Timeline

aheejin created this revision.Jul 8 2019, 3:12 AM
aheejin edited the summary of this revision. (Show Details)Jul 8 2019, 3:13 AM
aheejin updated this revision to Diff 208350.Jul 8 2019, 3:14 AM
  • Comment fix
aheejin edited the summary of this revision. (Show Details)Jul 8 2019, 3:20 AM
sbc100 accepted this revision.Jul 8 2019, 3:30 AM
sbc100 added inline comments.
lib/Target/WebAssembly/WebAssemblyAddMissingPrototypes.cpp
83 ↗(On Diff #208350)

Is there some kind of "any" construct in llvm that could take A.hasStructRetAttr as a predicate?

This revision is now accepted and ready to land.Jul 8 2019, 3:30 AM
aheejin updated this revision to Diff 208359.Jul 8 2019, 3:42 AM
  • Use llvm::any_of
aheejin marked an inline comment as done.Jul 8 2019, 3:42 AM
aheejin marked an inline comment as done.Jul 8 2019, 3:46 AM
aheejin added inline comments.
lib/Target/WebAssembly/WebAssemblyAddMissingPrototypes.cpp
83 ↗(On Diff #208350)

It does not seem to be possible to use hasStructRetAttr directly as an argument to llvm:any_of without some more binding functions, in which case I think it might be overly complicated. So I used a named lambda instead. (Unnamed lambda makes indentation kind of ugly here)

aheejin retitled this revision from Make sret parameter work with AddMissingPrototypes to [WebAssembly] Make sret parameter work with AddMissingPrototypes.Jul 8 2019, 4:03 AM
sbc100 accepted this revision.Jul 8 2019, 4:26 AM
sbc100 added inline comments.
lib/Target/WebAssembly/WebAssemblyAddMissingPrototypes.cpp
83 ↗(On Diff #208350)

No worries. I was mostly just curious. lgtm either way.

aheejin updated this revision to Diff 208368.Jul 8 2019, 4:55 AM
aheejin marked an inline comment as done.
  • sret argument should be the first one
lib/Target/WebAssembly/WebAssemblyAddMissingPrototypes.cpp
83 ↗(On Diff #208350)

Come to think of it, there can be at most one argument in this case and that should be the sret argument. Maybe we don't need a loop or any_of.

This revision was automatically updated to reflect the committed changes.