[PATCH] D66428: Change TargetLibraryInfo analysis passes to always require Function

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet added a comment.

Thx for working on this Teresa.



================
Comment at: lib/Analysis/GlobalsModRef.cpp:580
         if (auto *Call = dyn_cast<CallBase>(&I)) {
+          auto TLI = GetTLI(*Node->getFunction());
           if (isAllocationFn(Call, &TLI) || isFreeCall(Call, &TLI)) {
----------------
Would `auto&` be more appropriate here? I'm concerned about a possible copy - we're taking the address of `TLI` on next line.
Same in the rest of the code.


================
Comment at: lib/Transforms/IPO/GlobalOpt.cpp:2373
+        // so use default nullptr for that optional parameter.
+        Constant *New = ConstantFoldConstant(C, DL);
         if (New && New != C)
----------------
Maybe explicitly write the `nullptr`


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66428/new/

https://reviews.llvm.org/D66428