This is an archive of the discontinued LLVM Phabricator instance.

[CallSite removal][IPO] Move AbstractCallSite to its own header and change it to use CallBase instead of CallSite. NFCI
ClosedPublic

Authored by craig.topper on Apr 16 2020, 12:52 PM.

Details

Summary

CallSite will likely be removed soon, but AbstractCallSite serves a different purpose and won't be going away.

This patch moves it to its own header and switches it to internally store a CallBase* instead of a CallSite. The only interface changes are the removal of the getCallSite function and getCallBackUses now takes a CallBase&. These methods had only a few callers that were easy enough to update without needing a compatibility shim.

Diff Detail

Event Timeline

craig.topper created this revision.Apr 16 2020, 12:52 PM
dblaikie accepted this revision.Apr 16 2020, 1:08 PM

Looks good - I'd probably have done it differently (waited until all the CallSite uses were eliminated, deleted that code from CallSite.h, then renamed CallSite.h to AbstractCallSite.h) because I /think/ it'd present easier/better revision history, but that's only based on /really/ vague guesses about how smart/not-smart the change tracking stuff is - in any case, dealer's choice :)

This revision is now accepted and ready to land.Apr 16 2020, 1:08 PM

Looks good - I'd probably have done it differently (waited until all the CallSite uses were eliminated, deleted that code from CallSite.h, then renamed CallSite.h to AbstractCallSite.h) because I /think/ it'd present easier/better revision history, but that's only based on /really/ vague guesses about how smart/not-smart the change tracking stuff is - in any case, dealer's choice :)

That's good point. I mainly did it this way so I could remove the include of CallSite.h from Attributor so the compiler could tell me where all the uses of CallSite were.

Looks good - I'd probably have done it differently (waited until all the CallSite uses were eliminated, deleted that code from CallSite.h, then renamed CallSite.h to AbstractCallSite.h) because I /think/ it'd present easier/better revision history, but that's only based on /really/ vague guesses about how smart/not-smart the change tracking stuff is - in any case, dealer's choice :)

That's good point. I mainly did it this way so I could remove the include of CallSite.h from Attributor so the compiler could tell me where all the uses of CallSite were.

Yeah, figured it might be something like that - if you find it a good way to make progress, go for it. Super up to you :)

Looks good - I'd probably have done it differently (waited until all the CallSite uses were eliminated, deleted that code from CallSite.h, then renamed CallSite.h to AbstractCallSite.h) because I /think/ it'd present easier/better revision history, but that's only based on /really/ vague guesses about how smart/not-smart the change tracking stuff is - in any case, dealer's choice :)

That's good point. I mainly did it this way so I could remove the include of CallSite.h from Attributor so the compiler could tell me where all the uses of CallSite were.

Yeah, figured it might be something like that - if you find it a good way to make progress, go for it. Super up to you :)

Well removing the include didn't really help. Must be transitively included somewhere. So given that I'm going to just do the change to use CallBase in the implementation and not split the header.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 16 2020, 4:44 PM

Thanks for updating this!