[VPlan] Manage induction value creation using VPValues.

[fhahn]

Summary

This patch updates the induction value creation to use VPValues of
recipes to map the created values. This should bring is one step closer
to being able to optimize induction recipes directly in VPlan.

Currently widenIntOrFpInduction also generates vector values for a cast
of the induction, if it exists. Make this explicit by adding the cast
instruction to the values defined by the recipe.

[gilr]

Should the (final) goal be to move the whole phi-trunc-cast logic from code-gen to vplanning phase?

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

2093

Delete

2153–2154

Isn't extracting CastInst here from ID redundant now, given CastDef which wraps it?

2154

&& "error message"

9026

Can IsTrunc be replaced by dyn_cast<>? (and isa<> in print()?)

9027

Might be good to have named getters for the trunc and the cast defs.

9279

• Make this an early exit?
• Worth leaving a TODO for replacing the IF with an assertion once all scalar recipes are VPDef'ed and throwing away that callback.

[fhahn]

Address comments, thanks!

[fhahn]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

2154

I removed the assert as it should not be needed & also removed CastInst.

9027

Added helpers and removed IsTrunc field.

9279

The early exit is much better & I also added a TODO. thanks!

[fhahn]

In D92284#2508155, @gilr wrote:

Should the (final) goal be to move the whole phi-trunc-cast logic from code-gen to vplanning phase?

Yes I think so, I'm hoping we can simplify some of the logic handling inductions a bit by moving things to the planning phase.

[fhahn]

rebase

[fhahn]

llvm/lib/Transforms/Vectorize/VPlan.cpp

220

I had to add this as part of 3201274dea63, because otherwise there would be some failures for VPValue that were inductions. This is fixed with this patch, so this can be removed again. The OrigLoop field can also be removed again.

[fhahn]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

9279

I put up a patch to remove the getOrCreateVectorValues call and the whole VPCallback: D95383 :)

[fhahn]

Rebased & ping. @gilr does this look good after the recent changes? :)

[fhahn]

ping :)

[gilr]

LGTM with a nit. Thanks!

llvm/lib/Transforms/Vectorize/VPlan.h

943–944

Doesn't seem to be used anywhere (The start argument going into the VPUser)

[fhahn]

rebased & removed unnecessary leftover VPValue *Start;

Thanks!

[fhahn]

Another rebase on top of main

[uabelho]

Hi,

We've seen a crash with this patch.
Running

opt -o /dev/null bbi-53235.ll -loop-vectorize

yields

opt: ../lib/Transforms/Vectorize/VPlan.cpp:198: llvm::Value *llvm::VPTransformState::get(llvm::VPValue *, const llvm::VPIteration &): Assertion `hasVectorValue(Def, Instance.Part)' failed.

bbi-53235.ll1 KBDownload

[fhahn]

In D92284#2578584, @uabelho wrote:

Hi,

We've seen a crash with this patch.
Running

opt -o /dev/null bbi-53235.ll -loop-vectorize

yields

opt: ../lib/Transforms/Vectorize/VPlan.cpp:198: llvm::Value *llvm::VPTransformState::get(llvm::VPValue *, const llvm::VPIteration &): Assertion `hasVectorValue(Def, Instance.Part)' failed.

bbi-53235.ll1 KBDownload

Thanks for the heads-up. Should be fixed by c7ee57f1dccf

[uabelho]

In D92284#2578958, @fhahn wrote:

In D92284#2578584, @uabelho wrote:

Hi,

We've seen a crash with this patch.
Running

opt -o /dev/null bbi-53235.ll -loop-vectorize

yields

opt: ../lib/Transforms/Vectorize/VPlan.cpp:198: llvm::Value *llvm::VPTransformState::get(llvm::VPValue *, const llvm::VPIteration &): Assertion `hasVectorValue(Def, Instance.Part)' failed.

bbi-53235.ll1 KBDownload

Thanks for the heads-up. Should be fixed by c7ee57f1dccf

Yes, thanks!