This is an archive of the discontinued LLVM Phabricator instance.

Fix merging of small weak functions
ClosedPublic

Authored by nikic on May 13 2018, 7:10 AM.

Details

Summary

When two interposable functions are merged, we cannot replace
uses and have to emit calls to a common internal function. However,
writeThunk() will not actually emit a thunk if the function is too
small. This leaves us in a broken state where mergeTwoFunctions
already rewired the functions, but writeThunk doesn't do anything.

This patch changes the implementation so that:

  • writeThunk() does just that.
  • The direct replacement of calls is moved into mergeTwoFunctions() into the non-interposable case only.
  • isThunkProfitable() is extracted and will be called for the non-iterposable case always, and in the interposable case only if uses are still left after replacement.

This issue has been introduced in https://reviews.llvm.org/D34806,
where the code for checking thunk profitability has been moved.

Diff Detail

Event Timeline

nikic created this revision.May 13 2018, 7:10 AM
nikic edited the summary of this revision. (Show Details)May 13 2018, 7:11 AM

Thanks for fixing this!

nox added a subscriber: nox.May 13 2018, 8:12 AM
tamird added a subscriber: tamird.May 13 2018, 12:00 PM
This revision is now accepted and ready to land.May 15 2018, 12:48 AM
nox added a comment.May 15 2018, 4:31 AM

I rebased it against trunk.

This revision was automatically updated to reflect the committed changes.