This is an archive of the discontinued LLVM Phabricator instance.

[Pseudo probe] Duplicate probes in vectorized loop body.
ClosedPublic

Authored by hoy on Feb 14 2023, 6:21 PM.

Details

Summary

Prevoius pseudo probes were dropped out of a vectorized loop body during loop vectorization. This can result in the samples of the loop entry is used for the loop body, which in turn can cause undercounting of the loop iteration count. The undercounting can further prevent the loop from being vectorized in the next build. I'm fixing this by explicting allowing pseudo probes to be kept in the vectorized loop body, and by claiming a probe instruction is not "uniform", the vectorizer will duplicate it by the number of vector lanes.

For one internal service, I'm seeing the change causes the size increase of the .pseudoprobe section by 0.7%, which should count around 0.2% of the whole binary size.

Diff Detail

Event Timeline

hoy created this revision.Feb 14 2023, 6:21 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 14 2023, 6:21 PM
hoy requested review of this revision.Feb 14 2023, 6:21 PM
wenlei added inline comments.Feb 14 2023, 7:12 PM
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
1287–1293

Will a non-uniform instruction in loop hinder vectorization?

1288

Perhaps augment the comment to be more explicit about why we need to duplicate probes:

We want pseudo-probe to be duplicated for each unrolled iteration, so profiled loop trip count can be accurately accumulated instead of being under counted.

hoy added inline comments.Feb 15 2023, 9:11 AM
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
1287–1293

It doesn't. When it gets here, pseudo probes already passed legality checks. In the regression test, the loop can be vectorized with probes duplicated.

1288

Sounds good.

hoy updated this revision to Diff 497702.Feb 15 2023, 9:11 AM

Updating D144066: [Pseudo probe] Duplicate probes in vectorized loop body.

wenlei added inline comments.Feb 15 2023, 9:25 AM
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
1287–1293

How does this affect cost-benefit analysis? I saw it's used in LoopVectorizationCostModel::getInstructionCost.

hoy added inline comments.Feb 15 2023, 9:54 AM
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
1287–1293
wenlei accepted this revision.Feb 15 2023, 9:55 AM

lgtm, thanks.

This revision is now accepted and ready to land.Feb 15 2023, 9:55 AM
This revision was automatically updated to reflect the committed changes.
fhahn added a subscriber: fhahn.Feb 15 2023, 1:53 PM
fhahn added inline comments.
llvm/test/Transforms/SampleProfile/pseudo-probe-loop-vectorize.ll
2

It looks like this should test LV behavior and only runs LV, so it should be moved to the LoopVectorize test directory?

49

This metadata should not be needed, can you remove it to make the test more compact?

hoy added inline comments.Feb 15 2023, 2:04 PM
llvm/test/Transforms/SampleProfile/pseudo-probe-loop-vectorize.ll
2

Thanks for the suggestion. Does https://reviews.llvm.org/D144137 look good to you?