[llvm] r220536 - Make getDISubprogram(const Function *F) available in LLVM
Timur Iskhodzhanov
timurrrr at google.com
Mon Oct 27 13:02:01 PDT 2014
OK, so David Majnemer has agreed to fix it based on your discussion.
2014-10-24 13:59 GMT-07:00 David Blaikie <dblaikie at gmail.com>:
>
>
> On Fri, Oct 24, 2014 at 1:52 PM, David Majnemer <david.majnemer at gmail.com>
> wrote:
>>
>> On Fri, Oct 24, 2014 at 12:05 PM, David Blaikie <dblaikie at gmail.com>
>> wrote:
>>>
>>>
>>>
>>> On Fri, Oct 24, 2014 at 11:41 AM, Timur Iskhodzhanov
>>> <timurrrr at google.com> wrote:
>>>>
>>>> 2014-10-23 18:49 GMT-07:00 David Blaikie <dblaikie at gmail.com>:
>>>> >
>>>> >
>>>> > On Thu, Oct 23, 2014 at 6:21 PM, Timur Iskhodzhanov
>>>> > <timurrrr at google.com>
>>>> > wrote:
>>>> >>
>>>> >> 2014-10-23 18:10 GMT-07:00 David Blaikie <dblaikie at gmail.com>:
>>>> >> >
>>>> >> >
>>>> >> > On Thu, Oct 23, 2014 at 4:46 PM, Timur Iskhodzhanov
>>>> >> > <timurrrr at google.com>
>>>> >> > wrote:
>>>> >> >>
>>>> >> >> Author: timurrrr
>>>> >> >> Date: Thu Oct 23 18:46:28 2014
>>>> >> >> New Revision: 220536
>>>> >> >>
>>>> >> >> URL: http://llvm.org/viewvc/llvm-project?rev=220536&view=rev
>>>> >> >> Log:
>>>> >> >> Make getDISubprogram(const Function *F) available in LLVM
>>>> >> >>
>>>> >> >> Reviewed at http://reviews.llvm.org/D5950
>>>> >> >>
>>>> >> >> Modified:
>>>> >> >> llvm/trunk/include/llvm/IR/DebugInfo.h
>>>> >> >> llvm/trunk/lib/IR/DebugInfo.cpp
>>>> >> >> llvm/trunk/lib/Transforms/Scalar/SampleProfile.cpp
>>>> >> >>
>>>> >> >> Modified: llvm/trunk/include/llvm/IR/DebugInfo.h
>>>> >> >> URL:
>>>> >> >>
>>>> >> >>
>>>> >> >> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DebugInfo.h?rev=220536&r1=220535&r2=220536&view=diff
>>>> >> >>
>>>> >> >>
>>>> >> >>
>>>> >> >> ==============================================================================
>>>> >> >> --- llvm/trunk/include/llvm/IR/DebugInfo.h (original)
>>>> >> >> +++ llvm/trunk/include/llvm/IR/DebugInfo.h Thu Oct 23 18:46:28
>>>> >> >> 2014
>>>> >> >> @@ -961,6 +961,11 @@ public:
>>>> >> >> /// \brief Find subprogram that is enclosing this scope.
>>>> >> >> DISubprogram getDISubprogram(const MDNode *Scope);
>>>> >> >>
>>>> >> >> +/// \brief Find debug info for a given function.
>>>> >> >> +/// \returns a valid DISubprogram, if found. Otherwise, it
>>>> >> >> returns an
>>>> >> >> empty
>>>> >> >> +/// DISubprogram.
>>>> >> >> +DISubprogram getDISubprogram(const Function *F);
>>>> >> >> +
>>>> >> >> /// \brief Find underlying composite type.
>>>> >> >> DICompositeType getDICompositeType(DIType T);
>>>> >> >>
>>>> >> >>
>>>> >> >> Modified: llvm/trunk/lib/IR/DebugInfo.cpp
>>>> >> >> URL:
>>>> >> >>
>>>> >> >>
>>>> >> >> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=220536&r1=220535&r2=220536&view=diff
>>>> >> >>
>>>> >> >>
>>>> >> >>
>>>> >> >> ==============================================================================
>>>> >> >> --- llvm/trunk/lib/IR/DebugInfo.cpp (original)
>>>> >> >> +++ llvm/trunk/lib/IR/DebugInfo.cpp Thu Oct 23 18:46:28 2014
>>>> >> >> @@ -912,6 +912,24 @@ DISubprogram llvm::getDISubprogram(const
>>>> >> >> return DISubprogram();
>>>> >> >> }
>>>> >> >>
>>>> >> >> +DISubprogram llvm::getDISubprogram(const Function *F) {
>>>> >> >> + // We look for the first instr that has a debug annotation
>>>> >> >> leading
>>>> >> >> back
>>>> >> >> to F.
>>>> >> >> + const LLVMContext &Ctx = F->getParent()->getContext();
>>>> >> >> + for (auto &BB : *F) {
>>>> >> >> + for (auto &Inst : BB.getInstList()) {
>>>> >> >> + DebugLoc DLoc = Inst.getDebugLoc();
>>>> >> >> + if (DLoc.isUnknown())
>>>> >> >> + continue;
>>>> >> >> + const MDNode *Scope = DLoc.getScopeNode(Ctx);
>>>> >> >> + DISubprogram Subprogram = getDISubprogram(Scope);
>>>> >> >> + if (Subprogram.describes(F))
>>>> >> >> + return Subprogram;
>>>> >> >
>>>> >> >
>>>> >> > You've changed this in the moving - it used to return on the first
>>>> >> > Instruction with DebugInfo, now it keeps searching.
>>>> >>
>>>> >> Yep – we thought the original version was slightly incorrect?
>>>> >
>>>> >
>>>> > We? (sorry, perhaps I missed some IRC conversation, etc) - this was
>>>> > discussed in code review/on IRC when I described the idea to Diego
>>>> > originally. What problem were you seeing/thinking of?
>>>>
>>>> Me and David Majnemer.
>>>> There's no particular problem we were trying to fix, but we just kind
>>>> of thought the function can try a bit harder than it did.
>>>> Is this a wrong approach?
>>>
>>>
>>> There's no need for it to try harder. It's a (recently established)
>>> invariant that if a function has debug info (if the function appears in a
>>> CU's subprogram list) then all instructions in that function which have
>>> debug locs will chain up to that function and nowhere else.
>>>
>>> So as soon as you find a debug loc, either you've found the subprogram or
>>> you can stop searching because this function doesn't have debug info (it
>>> might have debug locs inlined into it from some debug-having function, but
>>> the function itself does not - these debuglocs are kept around so that if
>>> the non-debug-having function is later inlined into a debug-having function,
>>> that all works properly)
>>>
>>> Make sense?
>>
>>
>> This makes sense. I think it was the loop structure which seemed
>> irregular and lead us to believe that something might be wrong.
>>
>> I think it's a bit more clear when written like:
>> DISubprogram llvm::getDISubprogram(const Function *F) {
>> // We look for the first instr that has a debug annotation leading back
>> to F.
>> for (auto &BB : *F) {
>> auto Inst = std::find_if(BB.begin(), BB.end(), [](const Instruction
>> &Inst) {
>> return !Inst.getDebugLoc().isUnknown();
>> });
>
>
> & I here I had been trying to figure out a way to use STL algorithms here
> just to mess with Diego.
>
> Looks good to me.
>
> (I wonder if we should have some iterator utilities to make it easy to
> iterate over all instructions in a function without having to explicitly
> mention basic blocks, for example)
>
>>
>> if (Inst != BB.end()) {
>
>
> Maybe early-continue:
>
> if (Inst == BB.end())
> continue;
>
> ?
>
>>
>> DebugLoc DLoc = Inst->getDebugLoc();
>> const MDNode *Scope =
>> DLoc.getScopeNode(F->getParent()->getContext());
>> DISubprogram Subprogram = getDISubprogram(Scope);
>> return Subprogram.describes(F) ? Subprogram : DISubprogram();
>> }
>> }
>>
>> return DISubprogram();
>> }
>>
>>>
>>>
>>>>
>>>>
>>>> >> > (also, we could change this function to take a reference instead of
>>>> >> > a
>>>> >> > pointer - I was going to make that change when I saw this semantic
>>>> >> > change
>>>> >> > that caught me by surprise)
>>>> >> >
>>>> >> >>
>>>> >> >> + }
>>>> >> >> + }
>>>> >> >> +
>>>> >> >> + return DISubprogram();
>>>> >> >> +}
>>>> >> >> +
>>>> >> >> DICompositeType llvm::getDICompositeType(DIType T) {
>>>> >> >> if (T.isCompositeType())
>>>> >> >> return DICompositeType(T);
>>>> >> >>
>>>> >> >> Modified: llvm/trunk/lib/Transforms/Scalar/SampleProfile.cpp
>>>> >> >> URL:
>>>> >> >>
>>>> >> >>
>>>> >> >> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SampleProfile.cpp?rev=220536&r1=220535&r2=220536&view=diff
>>>> >> >>
>>>> >> >>
>>>> >> >>
>>>> >> >> ==============================================================================
>>>> >> >> --- llvm/trunk/lib/Transforms/Scalar/SampleProfile.cpp (original)
>>>> >> >> +++ llvm/trunk/lib/Transforms/Scalar/SampleProfile.cpp Thu Oct 23
>>>> >> >> 18:46:28
>>>> >> >> 2014
>>>> >> >> @@ -627,29 +627,6 @@ void SampleProfileLoader::propagateWeigh
>>>> >> >> }
>>>> >> >> }
>>>> >> >>
>>>> >> >> -/// \brief Locate the DISubprogram for F.
>>>> >> >> -///
>>>> >> >> -/// We look for the first instruction that has a debug annotation
>>>> >> >> -/// leading back to \p F.
>>>> >> >> -///
>>>> >> >> -/// \returns a valid DISubprogram, if found. Otherwise, it
>>>> >> >> returns an
>>>> >> >> empty
>>>> >> >> -/// DISubprogram.
>>>> >> >> -static const DISubprogram getDISubprogram(Function &F, const
>>>> >> >> LLVMContext
>>>> >> >> &Ctx) {
>>>> >> >> - for (auto &BI : F) {
>>>> >> >> - BasicBlock *BB = &BI;
>>>> >> >> - for (auto &Inst : BB->getInstList()) {
>>>> >> >> - DebugLoc DLoc = Inst.getDebugLoc();
>>>> >> >> - if (DLoc.isUnknown())
>>>> >> >> - continue;
>>>> >> >> - const MDNode *Scope = DLoc.getScopeNode(Ctx);
>>>> >> >> - DISubprogram Subprogram = getDISubprogram(Scope);
>>>> >> >> - return Subprogram.describes(&F) ? Subprogram :
>>>> >> >> DISubprogram();
>>>> >> >> - }
>>>> >> >> - }
>>>> >> >> -
>>>> >> >> - return DISubprogram();
>>>> >> >> -}
>>>> >> >> -
>>>> >> >> /// \brief Get the line number for the function header.
>>>> >> >> ///
>>>> >> >> /// This looks up function \p F in the current compilation unit
>>>> >> >> and
>>>> >> >> @@ -662,7 +639,7 @@ static const DISubprogram getDISubprogra
>>>> >> >> /// \returns the line number where \p F is defined. If it returns
>>>> >> >> 0,
>>>> >> >> /// it means that there is no debug information
>>>> >> >> available for
>>>> >> >> \p
>>>> >> >> F.
>>>> >> >> unsigned SampleProfileLoader::getFunctionLoc(Function &F) {
>>>> >> >> - const DISubprogram &S = getDISubprogram(F, *Ctx);
>>>> >> >> + DISubprogram S = getDISubprogram(&F);
>>>> >> >> if (S.isSubprogram())
>>>> >> >> return S.getLineNumber();
>>>> >> >>
>>>> >> >>
>>>> >> >>
>>>> >> >> _______________________________________________
>>>> >> >> llvm-commits mailing list
>>>> >> >> llvm-commits at cs.uiuc.edu
>>>> >> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>> >> >
>>>> >> >
>>>> >
>>>> >
>>>
>>>
>>
>
More information about the llvm-commits
mailing list