This is an archive of the discontinued LLVM Phabricator instance.

[llvm][NFC][CallSite] Removed CallSite from some implementation details.
ClosedPublic

Authored by mtrofin on Apr 15 2020, 4:57 PM.

Diff Detail

Event Timeline

mtrofin created this revision.Apr 15 2020, 4:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 15 2020, 4:57 PM
craig.topper added inline comments.Apr 15 2020, 5:14 PM
llvm/lib/Transforms/Utils/FunctionComparator.cpp
54–57

This looks like an unrelated formatting change?

589–591

Why do we need a const_cast? Aren't getCallingConv and getAttributes() both const methods?

llvm/tools/opt/AnalysisWrappers.cpp
42–44

Drop the extra parentheses around the dyn_cast?

mtrofin updated this revision to Diff 257914.Apr 15 2020, 5:46 PM
mtrofin marked 3 inline comments as done.

fixes

mtrofin marked an inline comment as done.Apr 15 2020, 5:47 PM
mtrofin added inline comments.
llvm/lib/Transforms/Utils/FunctionComparator.cpp
589–591

good point, fixed

craig.topper added inline comments.Apr 15 2020, 6:40 PM
llvm/lib/Transforms/Utils/FunctionComparator.cpp
179

If we use cast instead of dyn_cast we can drop this assert.

180

Can we just compare getOpcode() here? That seems more obvious about what it’s doing?

mtrofin updated this revision to Diff 257950.Apr 15 2020, 9:28 PM
mtrofin marked 4 inline comments as done.

fixes

This revision is now accepted and ready to land.Apr 15 2020, 10:01 PM
mtrofin added inline comments.Apr 15 2020, 10:31 PM
llvm/lib/Transforms/Utils/FunctionComparator.cpp
179

Thanks, in fact, the API should be refactored afterwards. Left a fixme. Also the assert comment may have been incorrect, come to think of it, the function seems to work fine for CallBrInst, too.

This revision was automatically updated to reflect the committed changes.