This is an archive of the discontinued LLVM Phabricator instance.

[SampleFDO] Stop letting findCalleeFunctionSamples return unrelated profiles for invoke instructions
ClosedPublic

Authored by wmi on Aug 10 2020, 10:02 AM.

Details

Summary

We see a warning of "No debug information found in function foo: Function profile not used" in a case. The function foo is called by an invoke instruction. It has no debug information because it has attribute((nodebug)) in the definition. It shouldn't have profile instance in the sample profile but compiler thinks it does, that turns out to be a compiler bug in findCalleeFunctionSamples. The bug is exposed when sample-profile-merge-inlinee is enabled recently.

Currently in findCalleeFunctionSamples, CalleeName is unset and is empty for invoke instruction. For empty CalleeName, findFunctionSamplesAt will treat the call as an indirect call and will return any inline instance profile at the same location as the instruction. That leads to a wrong profile being returned to function foo.

The patch set CalleeName when the instruction is an invoke.

Diff Detail

Event Timeline

wmi created this revision.Aug 10 2020, 10:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 10 2020, 10:02 AM
wmi requested review of this revision.Aug 10 2020, 10:02 AM
wenlei added inline comments.Aug 10 2020, 10:16 AM
llvm/lib/Transforms/IPO/SampleProfile.cpp
823

So the original differentiation between invoke and call here isn't intentional, but purely an implementation oversight?

wmi added inline comments.Aug 10 2020, 10:28 AM
llvm/lib/Transforms/IPO/SampleProfile.cpp
823

I think so. I don't see fundamental difference between call and invoke with regard to sampleFDO.

wenlei accepted this revision.Aug 10 2020, 10:44 AM

Thanks. LGTM.

This revision is now accepted and ready to land.Aug 10 2020, 10:44 AM
hoyFB accepted this revision.Aug 10 2020, 10:47 AM
hoyFB added inline comments.
llvm/lib/Transforms/IPO/SampleProfile.cpp
819

Does it return here for invokes that don't have debug info?

The fix looks good to me. We should not treat non-call instructions are indirect calls.

wmi added inline comments.Aug 10 2020, 10:50 AM
llvm/lib/Transforms/IPO/SampleProfile.cpp
819

No it doesn't. The invoke instruction has debug information. It is the definition of the function being invoked doesn't have debug information.

hoyFB added inline comments.Aug 10 2020, 10:51 AM
llvm/lib/Transforms/IPO/SampleProfile.cpp
819

Oh I see. Thanks for the fix!