[PATCH] D117140: [LV] Always create VPWidenCanonicalIVRecipe, optimize away later.

Ayal Zaks via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 18 08:48:51 PST 2022


Ayal added a comment.

In D117140#3251026 <https://reviews.llvm.org/D117140#3251026>, @fhahn wrote:

> In D117140#3250984 <https://reviews.llvm.org/D117140#3250984>, @Ayal wrote:
>
>> This is fine, thanks for accommodating, one last thought: does Canonical mean starting at zero, or possibly at EPResumeVal?
>
> I think checking for zero a the moment should suffice, as EPResumeVal will only be set as start value just before epilogue VPlan execution. This should also match the current behavior before the change (and  D117551 <https://reviews.llvm.org/D117551> which now contains the addition if `isCanonical`). Once a single plan contains both vector loops we might need some changes in that direction though.

Resetting start of canonicalIV to EPResumeVal just before epiloque VPlan execution might invalidate passes that relied on this value being zero, including removeRedundantCanonicalIVs()? A test would be good, either confirming or reassuring :-)



================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:351
+
+  if (WidenOriginalIV) {
+    WidenNewIV->replaceAllUsesWith(WidenOriginalIV);
----------------
Ayal wrote:
> ```  if (WidenOriginalIV && WidenNewIV) {
> ```
> 
> 
Perhaps suffice to


```
    if (WidenOriginalIV && WidenOriginalIV->isCanonical() &&
        WidenOriginalIV->getScalarType() == WidenNewIV->getScalarType()) {
      WidenNewIV->replaceAllUsesWith(WidenOriginalIV);
      WidenNewIV->eraseFromParent();
      return;
    }
```


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.h:50
+  /// Try to replace VPWidenCanonicalIVRecipes with the primary IV recipe, if it
+  /// exists.
+  static void removeRedundantVPWidenCanonicalIVRecipe(VPlan &Plan);
----------------
fhahn wrote:
> Ayal wrote:
> > Comment should be updated.
> Updated to reference canonical IV.
typo: "it  exists."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117140



More information about the llvm-commits mailing list