[PATCH] D116554: [VPlan] Add & use utility to check if only the first lane is used.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked 2 inline comments as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:1658
+
+bool VPUtils::onlyFirstLaneDemanded(VPValue *Def) {
+  return all_of(Def->users(), [](VPUser *U) {
----------------
david-arm wrote:
> 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?
> 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`)


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:2748
 
+struct VPUtils {
+  VPUtils() = delete;
----------------
david-arm wrote:
> 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?
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116554/new/

https://reviews.llvm.org/D116554