This is an archive of the discontinued LLVM Phabricator instance.

[SampleFDO] Compute BFI if the sample loader changes BPI in FSAFDO
ClosedPublic

Authored by xur on Nov 22 2021, 2:32 PM.

Details

Summary

The sample loader changes the branch probability but not BFI. Here we force a recompute of BFI if the branch probabilities are changed.
No performance changes with google benchmarks.

This patch also registers FSAFDO MIR passes probably.

Diff Detail

Event Timeline

xur created this revision.Nov 22 2021, 2:32 PM
xur requested review of this revision.Nov 22 2021, 2:32 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 22 2021, 2:32 PM
hoy accepted this revision.Nov 22 2021, 3:58 PM

LGTM, thanks.

This revision is now accepted and ready to land.Nov 22 2021, 3:58 PM
This revision was automatically updated to reflect the committed changes.
wenlei added inline comments.Nov 23 2021, 3:19 PM
llvm/lib/CodeGen/MIRSampleProfile.cpp
318

When profile loader leads to change, we should probably recalculate BFI. But should BFI calculation be done outside of sample loader given BFI is a separate analysis? The IR BFI is done outside of sample loader.

It feels to me that updating MBFI inside sample loader kind of breaks the componentization.

hoy added inline comments.Nov 23 2021, 3:40 PM
llvm/lib/CodeGen/MIRSampleProfile.cpp
318

Normally you would mark this pass to not preserve BFI and rely on subsequent LazyBFI pass to update BFI automatically. But it's a bit special here in that we are using the BFI right after here.

wenlei added inline comments.Nov 23 2021, 3:43 PM
llvm/lib/CodeGen/MIRSampleProfile.cpp
318

But it's a bit special here in that we are using the BFI right after here.

What is the use you're referring to? I only the use of MBFI->View which is not a real use.

I don't see it as a reason to introduce this tight coupling.