This is an archive of the discontinued LLVM Phabricator instance.

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

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



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

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.

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!


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.