[PATCH] D133666: pVplan] Add VPValue::isDefinedOutsideVectorRegions helper (NFC).

[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.h:3058
       return Rep->isUniform();
-    return false;
+    return VPV->isDefinedOutsideVectorRegions();
   }
----------------
reames wrote:
> With this placement, this is equivalent to return false.  The interesting case is the return true below.
> 
> Maybe:
> 
> ```
> if (VPV->isDefinedOutsideVectorRegion())
>   return true;
> auto *Def = VPV->getDef();
> assert(Def);  
> ...
> ```
Thanks, re-ordered the code!


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanValue.h:205
+  /// TODO: Also handle recipes defined in pre-header blocks.
+  bool isDefinedOutsideVectorRegions() const { return !getDef(); }
 };
----------------
Ayal wrote:
> Thanks for following up!
> Perhaps something like "isaVPLiveIn()" analogous to isa<VPLiveOut>()? Both are saying it's a wrapper of an IR value/user outside the scope of VPlan rather than a recipe inside this scope.
> More precise type information could be derived from the wrapped underlying IR, or implied assuming that VPlan is only fed-by and feeds scalars from/to IR.
The helper is intended to go beyond pure live-ins in the future. In particular, the current definition could also return true for recipes defined in the pre-header, as in the code below. I wasn't able to come up with a test case to trigger this code though for now. Does that make sense?

```
+bool VPValue::isDefinedOutsideVectorRegions() const {
+  const VPRecipeBase *Def = getDef();
+  if (!Def)
+    return true;
+  const VPBasicBlock *DefBlock = Def->getParent();
+  const VPlan &Plan = *DefBlock->getPlan();
+  return Plan.getEntry() == DefBlock;
+}
+
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133666