Page MenuHomePhabricator

[VPlan] Use recursive traversal iterator in VPSlotTracker.
ClosedPublic

Authored by fhahn on Apr 9 2021, 4:01 AM.

Details

Summary

This patch simplifies VPSlotTracker by using the recursive traversal
iterator to traverse all blocks in a VPlan in reverse post-order when
numbering VPValues in a plan.

This depends on a fix to RPOT (D100169). It also extends the traversal
unit tests to check RPOT.

Diff Detail

Unit TestsFailed

TimeTest
40 msx64 debian > LLVM.Feature/OperandBundles::function-attrs.ll
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/bin/opt -S -function-attrs < /mnt/disks/ssd0/agent/llvm-project/llvm/test/Feature/OperandBundles/function-attrs.ll | /mnt/disks/ssd0/agent/llvm-project/build/bin/FileCheck /mnt/disks/ssd0/agent/llvm-project/llvm/test/Feature/OperandBundles/function-attrs.ll
60 msx64 windows > LLVM.Feature/OperandBundles::function-attrs.ll
Script: -- : 'RUN: at line 1'; c:\ws\w16e2-1\llvm-project\premerge-checks\build\bin\opt.exe -S -function-attrs < C:\ws\w16e2-1\llvm-project\premerge-checks\llvm\test\Feature\OperandBundles\function-attrs.ll | c:\ws\w16e2-1\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w16e2-1\llvm-project\premerge-checks\llvm\test\Feature\OperandBundles\function-attrs.ll

Event Timeline

fhahn created this revision.Apr 9 2021, 4:01 AM
fhahn requested review of this revision.Apr 9 2021, 4:01 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 9 2021, 4:01 AM
Herald added a subscriber: vkmr. · View Herald Transcript
a.elovikov added inline comments.Apr 12 2021, 11:33 AM
llvm/lib/Transforms/Vectorize/VPlan.cpp
1243

I think that such usage would be the most common one (i.e. skipping the regions). Can we outline it into a separate helper somehow? The first idea coming to my mind is our custom VPlanRPOT class using the regular RPOT as a member and exposing iterators through filter_iterator, but maybe something nicer can be done as well.

fhahn updated this revision to Diff 337834.Apr 15 2021, 11:00 AM

rebased after changes to D100175

fhahn updated this revision to Diff 339729.Apr 22 2021, 11:08 AM

rebased to use utility from D101093 to create an iterator range that only contains VPBasicBlocks.

fhahn added inline comments.Apr 22 2021, 11:11 AM
llvm/lib/Transforms/Vectorize/VPlan.cpp
1243

I've put up D101093, which adds a helper to turn an iterator range into an iterator range that only includes VPBasicBlocks and also casts them to VPBasicBlock when de-referencing. It should support arbitrary iterators (RPO, depth first, post order), at the cost of having separate versions for const VPBasicBlock * and VPBasicBLock *

a.elovikov accepted this revision.Apr 22 2021, 12:22 PM

LGTM, thanks!

This revision is now accepted and ready to land.Apr 22 2021, 12:22 PM
fhahn updated this revision to Diff 340475.Apr 26 2021, 3:52 AM

rebased to use blocksOnly iterator.

This revision was automatically updated to reflect the committed changes.