[llvm] r302597 - [ProfileSummary] Make getProfileCount a non-static member function.

Easwaran Raman via llvm-commits llvm-commits at lists.llvm.org
Wed May 10 12:14:20 PDT 2017


Thanks!

On Wed, May 10, 2017 at 12:05 PM, Teresa Johnson <tejohnson at google.com>
wrote:

> Should be fixed by r302705.
> Teresa
>
> On Wed, May 10, 2017 at 11:09 AM, Easwaran Raman <eraman at google.com>
> wrote:
>
>> Thanks for letting me know. Teresa is testing a fix and will submit that
>> soon.
>>
>> On Wed, May 10, 2017 at 10:53 AM, Kostya Serebryany <kcc at google.com>
>> wrote:
>>
>>> This makes the ubsan bot sad, please fix or revert ASAP.
>>> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fas
>>> t/builds/4831/steps/check-llvm%20ubsan/logs/stdio
>>>
>>> lib/Analysis/ModuleSummaryAnalysis.cpp:235:33: runtime error: member call on null pointer of type 'llvm::ProfileSummaryInfo'
>>>     #0 0x216ffea in computeFunctionSummary(llvm::ModuleSummaryIndex&, llvm::Module const&, llvm::Function const&, llvm::BlockFrequencyInfo*, llvm::ProfileSummaryInfo*, bool, llvm::DenseSet<unsigned long, llvm::DenseMapInfo<unsigned long> >&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp:235:33
>>>
>>>
>>>
>>>
>>> On Tue, May 9, 2017 at 4:21 PM, Easwaran Raman via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>>
>>>> Author: eraman
>>>> Date: Tue May  9 18:21:10 2017
>>>> New Revision: 302597
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=302597&view=rev
>>>> Log:
>>>> [ProfileSummary] Make getProfileCount a non-static member function.
>>>>
>>>> This change is required because the notion of count is different for
>>>> sample profiling and getProfileCount will need to determine the
>>>> underlying profile type.
>>>>
>>>> Differential revision: https://reviews.llvm.org/D33012
>>>>
>>>> Modified:
>>>>     llvm/trunk/include/llvm/Analysis/ProfileSummaryInfo.h
>>>>     llvm/trunk/include/llvm/Transforms/Utils/Cloning.h
>>>>     llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp
>>>>     llvm/trunk/lib/Transforms/IPO/Inliner.cpp
>>>>     llvm/trunk/lib/Transforms/IPO/PartialInlining.cpp
>>>>     llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp
>>>>
>>>> Modified: llvm/trunk/include/llvm/Analysis/ProfileSummaryInfo.h
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/
>>>> Analysis/ProfileSummaryInfo.h?rev=302597&r1=302596&r2=302597&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- llvm/trunk/include/llvm/Analysis/ProfileSummaryInfo.h (original)
>>>> +++ llvm/trunk/include/llvm/Analysis/ProfileSummaryInfo.h Tue May  9
>>>> 18:21:10 2017
>>>> @@ -67,8 +67,8 @@ public:
>>>>    }
>>>>
>>>>    /// Returns the profile count for \p CallInst.
>>>> -  static Optional<uint64_t> getProfileCount(const Instruction
>>>> *CallInst,
>>>> -                                            BlockFrequencyInfo *BFI);
>>>> +  Optional<uint64_t> getProfileCount(const Instruction *CallInst,
>>>> +                                     BlockFrequencyInfo *BFI);
>>>>    /// \brief Returns true if \p F has hot function entry.
>>>>    bool isFunctionEntryHot(const Function *F);
>>>>    /// Returns true if \p F has hot function entry or hot call edge.
>>>>
>>>> Modified: llvm/trunk/include/llvm/Transforms/Utils/Cloning.h
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/
>>>> Transforms/Utils/Cloning.h?rev=302597&r1=302596&r2=302597&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- llvm/trunk/include/llvm/Transforms/Utils/Cloning.h (original)
>>>> +++ llvm/trunk/include/llvm/Transforms/Utils/Cloning.h Tue May  9
>>>> 18:21:10 2017
>>>> @@ -43,6 +43,7 @@ class InvokeInst;
>>>>  class Loop;
>>>>  class LoopInfo;
>>>>  class Module;
>>>> +class ProfileSummaryInfo;
>>>>  class ReturnInst;
>>>>
>>>>  /// Return an exact copy of the specified module
>>>> @@ -175,15 +176,17 @@ public:
>>>>    explicit InlineFunctionInfo(CallGraph *cg = nullptr,
>>>>                                std::function<AssumptionCache &(Function
>>>> &)>
>>>>                                    *GetAssumptionCache = nullptr,
>>>> +                              ProfileSummaryInfo *PSI = nullptr,
>>>>                                BlockFrequencyInfo *CallerBFI = nullptr,
>>>>                                BlockFrequencyInfo *CalleeBFI = nullptr)
>>>> -      : CG(cg), GetAssumptionCache(GetAssumptionCache),
>>>> CallerBFI(CallerBFI),
>>>> -        CalleeBFI(CalleeBFI) {}
>>>> +      : CG(cg), GetAssumptionCache(GetAssumptionCache), PSI(PSI),
>>>> +        CallerBFI(CallerBFI), CalleeBFI(CalleeBFI) {}
>>>>
>>>>    /// CG - If non-null, InlineFunction will update the callgraph to
>>>> reflect the
>>>>    /// changes it makes.
>>>>    CallGraph *CG;
>>>>    std::function<AssumptionCache &(Function &)> *GetAssumptionCache;
>>>> +  ProfileSummaryInfo *PSI;
>>>>    BlockFrequencyInfo *CallerBFI, *CalleeBFI;
>>>>
>>>>    /// StaticAllocas - InlineFunction fills this in with all static
>>>> allocas that
>>>>
>>>> Modified: llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/
>>>> ModuleSummaryAnalysis.cpp?rev=302597&r1=302596&r2=302597&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp (original)
>>>> +++ llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp Tue May  9
>>>> 18:21:10 2017
>>>> @@ -232,7 +232,7 @@ computeFunctionSummary(ModuleSummaryInde
>>>>          }
>>>>          // We should have named any anonymous globals
>>>>          assert(CalledFunction->hasName());
>>>> -        auto ScaledCount = ProfileSummaryInfo::getProfileCount(&I,
>>>> BFI);
>>>> +        auto ScaledCount = PSI->getProfileCount(&I, BFI);
>>>>          auto Hotness = ScaledCount ? getHotness(ScaledCount.getValue(),
>>>> PSI)
>>>>                                     : CalleeInfo::HotnessType::Unknown;
>>>>
>>>>
>>>> Modified: llvm/trunk/lib/Transforms/IPO/Inliner.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transform
>>>> s/IPO/Inliner.cpp?rev=302597&r1=302596&r2=302597&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- llvm/trunk/lib/Transforms/IPO/Inliner.cpp (original)
>>>> +++ llvm/trunk/lib/Transforms/IPO/Inliner.cpp Tue May  9 18:21:10 2017
>>>> @@ -502,7 +502,7 @@ inlineCallsImpl(CallGraphSCC &SCC, CallG
>>>>          std::swap(CallSites[i--], CallSites[--FirstCallInSCC]);
>>>>
>>>>    InlinedArrayAllocasTy InlinedArrayAllocas;
>>>> -  InlineFunctionInfo InlineInfo(&CG, &GetAssumptionCache);
>>>> +  InlineFunctionInfo InlineInfo(&CG, &GetAssumptionCache, PSI);
>>>>
>>>>    // Now that we have all of the call sites, loop over them and inline
>>>> them if
>>>>    // it looks profitable to do so.
>>>> @@ -872,7 +872,7 @@ PreservedAnalyses InlinerPass::run(LazyC
>>>>        // Setup the data structure used to plumb customization into the
>>>>        // `InlineFunction` routine.
>>>>        InlineFunctionInfo IFI(
>>>> -          /*cg=*/nullptr, &GetAssumptionCache,
>>>> +          /*cg=*/nullptr, &GetAssumptionCache, PSI,
>>>>            &FAM.getResult<BlockFrequencyAnalysis>(*(CS.getCaller())),
>>>>            &FAM.getResult<BlockFrequencyAnalysis>(Callee));
>>>>
>>>>
>>>> Modified: llvm/trunk/lib/Transforms/IPO/PartialInlining.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transform
>>>> s/IPO/PartialInlining.cpp?rev=302597&r1=302596&r2=302597&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- llvm/trunk/lib/Transforms/IPO/PartialInlining.cpp (original)
>>>> +++ llvm/trunk/lib/Transforms/IPO/PartialInlining.cpp Tue May  9
>>>> 18:21:10 2017
>>>> @@ -473,7 +473,7 @@ Function *PartialInlinerImpl::unswitchFu
>>>>               << ore::NV("Callee", F) << " partially inlined into "
>>>>               << ore::NV("Caller", CS.getCaller()));
>>>>
>>>> -    InlineFunctionInfo IFI(nullptr, GetAssumptionCache);
>>>> +    InlineFunctionInfo IFI(nullptr, GetAssumptionCache, PSI);
>>>>      InlineFunction(CS, IFI);
>>>>      NumPartialInlining++;
>>>>      // update stats
>>>>
>>>> Modified: llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transform
>>>> s/Utils/InlineFunction.cpp?rev=302597&r1=302596&r2=302597&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp (original)
>>>> +++ llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp Tue May  9
>>>> 18:21:10 2017
>>>> @@ -1396,11 +1396,12 @@ static void updateCallerBFI(BasicBlock *
>>>>  /// Update the branch metadata for cloned call instructions.
>>>>  static void updateCallProfile(Function *Callee, const
>>>> ValueToValueMapTy &VMap,
>>>>                                const Optional<uint64_t>
>>>> &CalleeEntryCount,
>>>> -                              const Instruction *TheCall) {
>>>> +                              const Instruction *TheCall,
>>>> +                              ProfileSummaryInfo *PSI) {
>>>>    if (!CalleeEntryCount.hasValue() || CalleeEntryCount.getValue() < 1)
>>>>      return;
>>>>    Optional<uint64_t> CallSiteCount =
>>>> -      ProfileSummaryInfo::getProfileCount(TheCall, nullptr);
>>>> +      PSI ? PSI->getProfileCount(TheCall, nullptr) : None;
>>>>    uint64_t CallCount =
>>>>        std::min(CallSiteCount.hasValue() ? CallSiteCount.getValue() :
>>>> 0,
>>>>                 CalleeEntryCount.getValue());
>>>> @@ -1423,16 +1424,16 @@ static void updateCallProfile(Function *
>>>>  /// The callsite's block count is subtracted from the callee's
>>>> function entry
>>>>  /// count.
>>>>  static void updateCalleeCount(BlockFrequencyInfo *CallerBFI,
>>>> BasicBlock *CallBB,
>>>> -                              Instruction *CallInst, Function *Callee)
>>>> {
>>>> +                              Instruction *CallInst, Function *Callee,
>>>> +                              ProfileSummaryInfo *PSI) {
>>>>    // If the callee has a original count of N, and the estimated count
>>>> of
>>>>    // callsite is M, the new callee count is set to N - M. M is
>>>> estimated from
>>>>    // the caller's entry count, its entry block frequency and the block
>>>> frequency
>>>>    // of the callsite.
>>>>    Optional<uint64_t> CalleeCount = Callee->getEntryCount();
>>>> -  if (!CalleeCount.hasValue())
>>>> +  if (!CalleeCount.hasValue() || !PSI)
>>>>      return;
>>>> -  Optional<uint64_t> CallCount =
>>>> -      ProfileSummaryInfo::getProfileCount(CallInst, CallerBFI);
>>>> +  Optional<uint64_t> CallCount = PSI->getProfileCount(CallInst,
>>>> CallerBFI);
>>>>    if (!CallCount.hasValue())
>>>>      return;
>>>>    // Since CallSiteCount is an estimate, it could exceed the original
>>>> callee
>>>> @@ -1635,9 +1636,10 @@ bool llvm::InlineFunction(CallSite CS, I
>>>>        updateCallerBFI(OrigBB, VMap, IFI.CallerBFI, IFI.CalleeBFI,
>>>>                        CalledFunc->front());
>>>>
>>>> -    updateCallProfile(CalledFunc, VMap, CalledFunc->getEntryCount(),
>>>> TheCall);
>>>> +    updateCallProfile(CalledFunc, VMap, CalledFunc->getEntryCount(),
>>>> TheCall,
>>>> +                      IFI.PSI);
>>>>      // Update the profile count of callee.
>>>> -    updateCalleeCount(IFI.CallerBFI, OrigBB, TheCall, CalledFunc);
>>>> +    updateCalleeCount(IFI.CallerBFI, OrigBB, TheCall, CalledFunc,
>>>> IFI.PSI);
>>>>
>>>>      // Inject byval arguments initialization.
>>>>      for (std::pair<Value*, Value*> &Init : ByValInit)
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>
>>>
>>>
>>
>
>
> --
> Teresa Johnson |  Software Engineer |  tejohnson at google.com |
> 408-460-2413 <(408)%20460-2413>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170510/4743b5dc/attachment.html>


More information about the llvm-commits mailing list