This is an archive of the discontinued LLVM Phabricator instance.

[VPlan] Don't consider VPWidenCanonicalIVRecipe phi-like.
ClosedPublic

Authored by fhahn on Jan 1 2022, 4:17 AM.

Details

Summary

VPWidenCanonicalIVRecipe does not create PHI instructions, so it does
not need to be placed in the phi section of a VPBasicBlock.

Discussed D113223.

Diff Detail

Event Timeline

fhahn created this revision.Jan 1 2022, 4:17 AM
fhahn requested review of this revision.Jan 1 2022, 4:17 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 1 2022, 4:17 AM
Herald added a subscriber: vkmr. · View Herald Transcript
Ayal added inline comments.Jan 1 2022, 2:25 PM
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
8445–8451

How about ironing this a bit, while we're here, e.g.:

// Introduce the early-exit compare IV <= BTC to form header block mask.
// This is used instead of IV < TC because TC may wrap, unlike BTC.
// Start by constructing the desired canonical IV in the header block as its first non-phi instructions.
assert(CM.foldTailByMasking() && "must fold the tail");
VPBasicBlock *HeaderVPBB = Plan->getEntry()->getEntryBasicBlock();
auto NewInsertionPoint = HeaderVPBB->getFirstNonPhi();

VPValue *IV = nullptr;
if (Legal->getPrimaryInduction())
  IV = Plan->getOrAddVPValue(Legal->getPrimaryInduction());
else {
  auto *IVRecipe = new VPWidenCanonicalIVRecipe();
  HeaderVPBB->insert(IVRecipe, NewInsertionPoint);
  IV = IVRecipe;
}

assert(Builder.getInsertBlock() == HeaderVPBB && "..."); // if desired?
VPBuilder::InsertPointGuard Guard(Builder);
Builder.setInsertPoint(HeaderVPBB, NewInsertionPoint);
fhahn updated this revision to Diff 396883.Jan 1 2022, 2:38 PM

Apply suggestion, thanks!

fhahn marked an inline comment as done.Jan 1 2022, 2:41 PM
fhahn added inline comments.
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
8445–8451

This now means that the compare recipe always gets created in the header VPBB whereas before it may be created in a different VPBB (but still reachable from the header through single branches). This causes some slight changes in the order the IR is generated, but overall seems like a nice simplification. I updated the patch.

Ayal accepted this revision.Jan 2 2022, 12:54 AM

This looks good to me, thanks!

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
8445–8451

Well, the documentation claims that the desired canonical IV is constructed "in the header block"

The distinct VPBB's seem to be equivalent, necessarily? Such as the entry of a replicating region and its single predecessor that appear in vplan-sink-scalars-and-merge.ll, which should be fused during code-gen.

This revision is now accepted and ready to land.Jan 2 2022, 12:54 AM
This revision was landed with ongoing or failed builds.Jan 2 2022, 4:49 AM
This revision was automatically updated to reflect the committed changes.
fhahn marked an inline comment as done.