[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