Page MenuHomePhabricator

[LoopVectorize] NFCI: BuildVPlansWithVPRecipes to include ScalableVFs.
AbandonedPublic

Authored by sdesmalen on Feb 11 2021, 2:00 PM.

Details

Reviewers
None
Summary

This patch makes sure that the loopvectorizer generates VPlans for
all power of two VFs upto the provided maximum VFs (can be both fixed
and/or scalable). This patch is intended to be a non-functional change,
because selectVectorizationFactor does not automatically pick any of
the scalable VPlans yet.

This patch is a preparatory patch with the ultimate goal of making
computeMaxVF() return both a max fixed VF and a max scalable VF,
so that selectVectorizationFactor() can pick the most cost-effective
vectorization factor.

Diff Detail

Event Timeline

sdesmalen created this revision.Feb 11 2021, 2:00 PM
sdesmalen requested review of this revision.Feb 11 2021, 2:00 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 11 2021, 2:00 PM
Herald added a subscriber: vkmr. · View Herald Transcript
sdesmalen updated this revision to Diff 324661.Feb 18 2021, 8:45 AM

Simplified genFeasibleVFCandidates

nasherm added a subscriber: nasherm.Mar 2 2021, 3:04 AM
nasherm added inline comments.
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
2146

There is a code path that's being travelled down after your genFeasibleVFCandidates function that causes a runtime crash where VF elements
. The issue is caused by the fact that this code path assumes that the supplied MinFactor and MaxFactor, of type ElementCount, will be in the Scalars map. For illustration here's the piece of code that triggers this error code path

Optional<VectorizationFactor>
7595 LoopVectorizationPlanner::plan(ElementCount UserVF, unsigned UserIC) {
...
7640   genFeasibleVFCandidates(CM, VFCandidates, MinFactors, MaxFactors);
7641   for (auto &VF : VFCandidates) {
7642     // Collect Uniform and Scalar instructions after vectorization with VF.
7643     CM.collectUniformsAndScalars(VF);
7644 
7645     // Collect the instructions (and their associated costs) that will be more
7646     // profitable to scalarize.
7647     if (VF.isVector())
7648       CM.collectInstsToScalarize(VF);
7649   }
7650 
7651   CM.collectInLoopReductions();
7652 
7653   buildVPlansWithVPRecipes(MinFactors, MaxFactors); // <-- code path starts from this call
7654  ....

The buildVPlansWithVPRecipes function will traverse MinFactors->MaxFactors, similar to the genFeasibleVFCandidates candidates, but it will not perform the same !CM.canVectorizeReductions check which allows for the early breakout. As a consequence it will eventually hit this function

8374 bool VPRecipeBuilder::shouldWiden(Instruction *I, VFRange &Range) const {
8375   assert(!isa<BranchInst>(I) && !isa<PHINode>(I) && !isa<LoadInst>(I) &&
8376          !isa<StoreInst>(I) && "Instruction should have been handled earlier");
8377   // Instruction should be widened, unless it is scalar after vectorization,
8378   // scalarization is profitable or it is predicated.
8379   auto WillScalarize = [this, I](ElementCount VF) -> bool {
8380     return CM.isScalarAfterVectorization(I, VF) ||
8381            CM.isProfitableToScalarize(I, VF) ||
8382            CM.isScalarWithPredication(I, VF);
8383   };
8384   return !LoopVectorizationPlanner::getDecisionAndClampRange(WillScalarize,
8385                                                              Range);
8386 }

The call to CM.isScalarVectorization is the source of the error where the assert call is causing the crash. A simple, but perhaps inelegant solution, would be to add the !CM.canVectorizeReductions to the list of checks.

sdesmalen updated this revision to Diff 327908.Mar 3 2021, 1:47 PM
  • Rebased patch.
  • Removed call to canVectorizeReductions in GenerateRange, as this is now handled in computeFeasibleMaxVF.
sdesmalen abandoned this revision.Mar 12 2021, 7:40 AM

Abandoning in favour of a new patch series (see D98509)