This is an archive of the discontinued LLVM Phabricator instance.

Add !callees metadata
ClosedPublic

Authored by mssimpso on Aug 31 2017, 2:08 PM.

Details

Summary

This patch adds a new kind of metadata that indicates the possible callees of indirect calls.

Diff Detail

Repository
rL LLVM

Event Timeline

mssimpso created this revision.Aug 31 2017, 2:08 PM
hfinkel added a subscriber: hfinkel.Sep 2 2017, 2:57 PM

I don't like the term "targets", in part because that term is generally used to refer to the target architecture. How about "callees"?

docs/LangRef.rst
4883 ↗(On Diff #113462)

run-time -> runtime

4883 ↗(On Diff #113462)

If `targets` metadata is attached to a call site, and any callee is not among the set of functions provided by the metadata, the behavior is undefined.

I don't like the term "targets", in part because that term is generally used to refer to the target architecture. How about "callees"?

Yes, I like "callees" much better too. Thanks very much for the suggestions!

mssimpso updated this revision to Diff 114220.Sep 7 2017, 11:37 AM
mssimpso marked 2 inline comments as done.
mssimpso retitled this revision from Add !targets metadata to Add !callees metadata.
mssimpso edited the summary of this revision. (Show Details)

Addressed Hal's comments. Thanks!

hfinkel added inline comments.Sep 7 2017, 3:27 PM
docs/LangRef.rst
4883 ↗(On Diff #114220)

You can remove this statement entirely: "It indicates a known set of functions that a call or invoke instruction could possibly target at runtime."

The next sentence says that same thing in a more precise way.

4886 ↗(On Diff #114220)

You can remove this statement: "The compiler is free to optimize call sites under this assumption
(e.g., by performing indirect call promotion)."

The freedom for the compiler to optimize is implied by the preceding sentence. If you feel that stating an example optimization helps make the intent clearer, then I think that's fine. Maybe phrase this as:

The intent of this metadata is to facilitate optimizations such as indirect call promotion.

mssimpso updated this revision to Diff 114369.Sep 8 2017, 8:29 AM
mssimpso marked 2 inline comments as done.

Addressed Hal's comments. Thanks again!

hfinkel accepted this revision.Sep 8 2017, 8:32 AM

LGTM

docs/LangRef.rst
4884 ↗(On Diff #114369)

indirect call promotion -> indirect-call promotion

This revision is now accepted and ready to land.Sep 8 2017, 8:32 AM
This revision was automatically updated to reflect the committed changes.