[PATCH] D79917: [llvm][NFC] Cleanup uses of std::function in Inlining-related APIs
David Blaikie via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu May 14 19:03:24 PDT 2020
dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.
Looks good at a glance to me - welcome to make changes like that without review in the future.
================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1907-1911
for (Instruction &I : NewBlock) {
if (auto *II = dyn_cast<IntrinsicInst>(&I))
if (II->getIntrinsicID() == Intrinsic::assume)
- (*IFI.GetAssumptionCache)(*Caller).registerAssumption(II);
+ IFI.GetAssumptionCache(*Caller).registerAssumption(II);
}
----------------
Aside: This set of {} seems sort of arbitrary - given the larger for loop outside this doesn't use {} and the ifs inside don't... why this particular level of nesting with {}? Could remove those in a separate patch if you like
================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:2499
AssumptionCache *AC =
- IFI.GetAssumptionCache ? &(*IFI.GetAssumptionCache)(*Caller) : nullptr;
+ IFI.GetAssumptionCache ? &(IFI.GetAssumptionCache(*Caller)) : nullptr;
auto &DL = Caller->getParent()->getDataLayout();
----------------
This still seems to have unnecessary parentheses, instead: `&IFI.GetAssumptionCache(*Caller)` should be fine. (few other cases of similarly unnecessary () in other similar places in this file you could remove, since you're moving them around anyway)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79917/new/
https://reviews.llvm.org/D79917
More information about the llvm-commits
mailing list