Page MenuHomePhabricator

[LV] Move exit cond simplification to separate transform.
Needs ReviewPublic

Authored by fhahn on Oct 1 2022, 12:42 PM.

Details

Reviewers
Ayal
reames
gilr
Summary

This sets the stage for D133017 by moving out the code that performs
VPlan based simplifications to a separate transform that takes the
chosen VF & UF as arguments.

The main advantage is that this transform runs before any changes to
the CFG are being made. This allows using SCEV without worrying about
making queries while the IR is in an incomplete state.

Note that this patch switches the reasoning to use SCEV, but still only
simplifies loops with constant trip counts. Using SCEV here is needed to
access the backedge taken count, because the trip count IR value has not
been created yet.

Diff Detail

Event Timeline

fhahn created this revision.Oct 1 2022, 12:42 PM
fhahn requested review of this revision.Oct 1 2022, 12:42 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 1 2022, 12:42 PM
Ayal added inline comments.Oct 2 2022, 7:32 AM
llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
464

TripCountV indeed hasn't been created yet, but perhaps there's a simpler way to check this condition, e.g., by checking if PSE.getBackedgeCount() is a constant greater than VF*UF-1, instead of incrementing it by 1 of type IdxTy; or trying to call SE.getSmallConstantTripCount(L); or recording it in VPlan, possibly in CanonicalIV recipe.

474

Worth an unconditional branch VPInstruction instead of the effort to generate a redundant True? Can be fixed independent of this patch, which is working a bit harder to generate it.

llvm/lib/Transforms/Vectorize/VPlanTransforms.h
66

Missing documentation.

llvm/test/Transforms/LoopVectorize/lcssa-crashes.ll
16

Is this a regression from previously folding the conditional branch?

Ayal added inline comments.Oct 2 2022, 7:40 AM
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
7623

nit: can drop "Concrete".

Restrict the VF range of BestPlan to include only BestVF?

When printing BestPlan would be good to now indicate that it applies to BestUF only.