This is an archive of the discontinued LLVM Phabricator instance.

Provide reason messages for unviable inlining
ClosedPublic

Authored by yrouban on Jan 23 2019, 2:15 AM.

Details

Summary

InlineCost's isInlineViable() is changed to return InlineResult instead of bool. This provides messages for failure reasons.
This allows to get more specific messages for cases where callsites are not viable for inlining.

Diff Detail

Repository
rC Clang

Event Timeline

yrouban created this revision.Jan 23 2019, 2:15 AM
xbolva00 added inline comments.Jan 25 2019, 6:53 AM
lib/Analysis/InlineCost.cpp
2028 ↗(On Diff #183045)

getNever should take a string

yrouban marked an inline comment as done.Jan 25 2019, 7:44 AM
yrouban added inline comments.
lib/Analysis/InlineCost.cpp
2028 ↗(On Diff #183045)

InlineResult is implicitly converted to char* Reason.
Do you propose explicit IsViable.message?

xbolva00 added inline comments.Jan 25 2019, 7:56 AM
lib/Analysis/InlineCost.cpp
2028 ↗(On Diff #183045)

Yes, readability would be better

anemet added a subscriber: anemet.Jan 25 2019, 9:22 AM

This is great, please add some tests or check for remarks in existing tests (e.g. for the recursive case).

yrouban updated this revision to Diff 183806.Jan 28 2019, 12:51 AM

Addressed comments:

  • added a simple test
  • changed implicit conversion to char* to explicit field access IsViable.message

@anemet, @xbolva00, please take a look once again.

xbolva00 accepted this revision.Jan 31 2019, 3:27 AM

Looks fine for me

This revision is now accepted and ready to land.Jan 31 2019, 3:27 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 1 2019, 2:44 AM