[VPlan] Use VPlan to check if only the first lane is used.

[fhahn]

Summary

This removes the remaining dependence on LoopVectorizationCostModel from
buildScalarSteps and is required so it can be moved out of ILV.

It also improves allows us to remove a few unneeded instructions.

[david-arm]

llvm/lib/Transforms/Vectorize/VPlan.cpp

1653

Hi @fhahn, on the face of it this looks like a reasonable change, but I'm just wondering if we're now going to have to update two places in the code when someone makes a change to isUniformAfterVectorization. For example, can we guarantee that for all cases where isUniformAfterVectorization returns true we will generate a VPReplicateRecipe that is uniform?

During creations of vplans is there a way to assert that if isUniformAfterVectorization returns true then VPUtils::onlyFirstLaneDemanded should also be true?

llvm/lib/Transforms/Vectorize/VPlan.h

2842

Given you can't create an instance of this, does it even need to be a struct? Could you just use a namespace VPUtils instead?

[fhahn]

Address comments, thanks! Move to namespace and also consider VPWidenMemoryInstructionRecipes as uniform, if they are consecutive and the def is used as pointer only.

[fhahn]

llvm/lib/Transforms/Vectorize/VPlan.cpp

1653

Hi @fhahn, on the face of it this looks like a reasonable change, but I'm just wondering if we're now going to have to update two places in the code when someone makes a change to isUniformAfterVectorization. For example, can we guarantee that for all cases where isUniformAfterVectorization returns true we will generate a VPReplicateRecipe that is uniform?

I don't think we need to update this function if someone makes changes to isUniformAfterVectorization. We would only need to update the function if 'uniform-after-vectorization' instructions are materialized differently in VPlan, i.e. if other recipes than uniform VPReplicateRecipes are used for instructions that are isUniformAfterVectorization.

In a way, this check here should be more accurate, because it looks at the recipes which are responsible for the actual generated code. I think there are 2 remaining places where isUniformAfterVectorization is still used during recipe execution, but both cases should not interact with users of int/fp inductions. I put up 2 patches to replace those uses as well D116654 & D116656. This also highlighted a missing case: VPWidenMemoryInstructionRecipes also only demand the first lane, if they are consecutive and the def is only used as pointer argument. This is only relevant for pointer inductions.

During creations of vplans is there a way to assert that if isUniformAfterVectorization returns true then VPUtils::onlyFirstLaneDemanded should also be true?

I think that would be possible, but I am not sure if it is necessary. With D116654 & D116656, all uses of isUniformAfterVectorization have been replaced. If any additional recipe needs to make use of a uniform-after-vectorization check, we should express it explicitly in the recipe (or use onlyFirstLaneDemanded)

llvm/lib/Transforms/Vectorize/VPlan.h

2842

I think VPBlockUtils uses a struct in a similar fashion, but I am not aware of any benefits of using a struct. Moved to namespace.

[Ayal]

Worth having RecipeBase indicate if it's using first lane only, per operand and/or for all operands, in order to associate this logic within each recipe?

UAV is propagated at the outset and its results are encoded in isUniform replicating recipes upon VPlan construction; should Blend recipe also record if it's blending uniform values or not, when constructed, instead of looking through it recursively here?

[fhahn]

In D116554#3239783, @Ayal wrote:

Worth having RecipeBase indicate if it's using first lane only, per operand and/or for all operands, in order to associate this logic within each recipe?

I moved the helpers to D116123 so they can be used there to start with. I also applied the suggestion there and removed the definitions here. Unfortunately the change here cannot be applied before D116123 because it requires separating out the step-vector part from the induction recipes first.

UAV is propagated at the outset and its results are encoded in isUniform replicating recipes upon VPlan construction; should Blend recipe also record if it's blending uniform values or not, when constructed, instead of looking through it recursively here?

I am not sure if it is worth adding a new field at the moment, given that it should be very cheap to find out given the existing information and it reduces the amount of state we need to carry. I think it might be good as a follow-up, which can also use it during codegen of the recipe, so we only generate a scalar select.

[Ayal]

This looks fine, with a minor comment.

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

2604–2605

Update comment as EntryVal is no longer checked? In fact, it need not be passed in.

[fhahn]

Move back the vputils::onlyFirstLaneUsed definitions here from D116123. Also add onlyFirstLaneUsed to VPCanoicalIVPHIRecipe, as suggested in D116123.

[Ayal]

This is fine, remove EntryVal from buildScalarSteps() and update comment?

[fhahn]

Thank Ayal! I rebased the patch and remove the stale comment about EntryVal. I plan to land the change soon.