[PATCH] D95676: [VPlan][NFC] Introduce constructor for VPIteration

Sander de Smalen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 2 05:26:41 PST 2021


sdesmalen added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:101
+
+  VPIteration() : Part(0), Lane(0) {}
+  VPIteration(unsigned Part, unsigned Lane) : Part(Part), Lane(Lane) {}
----------------
david-arm wrote:
> fhahn wrote:
> > sdesmalen wrote:
> > > From what I can see, most uses of VPIteration() in this patch are where it explicitly tries to get the first iteration. I think it's better to drop the implicit '0, 0' constructor, so that all VPIteration objects are explicitly constructed with a specific lane and part. Perhaps you can add a named method to return the first lane, e.g.
> > > 
> > >   static VPIteration getFirstLane() { return VPIteration(0, 0); }
> > I also think it's better to just explicitly instantiate the VPIteration, rather than having the default constructor defaulting to the first lane. Not sure about adding a helper, because `VPIteration(0, 0)` is shorter than ` VPIteration::getFirstLane` and as explicit, especially because all other places also pass lane & part explicitly.
> Hi @sdesmalen, hope you don't mind but I chose to go with @fhahn's suggestion of just using VPIteration(0, 0)
Sure I'm happy with that.


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

https://reviews.llvm.org/D95676



More information about the llvm-commits mailing list