[PATCH] D57598: [VPLAN] Determine Vector Width programmatically.

Francesco Petrogalli via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 14 13:49:41 PDT 2019


fpetrogalli marked 4 inline comments as done.
fpetrogalli added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorizationPlanner.h:233-234
   /// VF and its cost.
-  VectorizationFactor planInVPlanNativePath(bool OptForSize, unsigned UserVF);
+  VectorizationFactor planInVPlanNativePath(const bool OptForSize,
+                                            const unsigned UserVF);
 
----------------
Meinersbur wrote:
> fpetrogalli wrote:
> > Meinersbur wrote:
> > > [nit] It it somewhat unusual to pass by-value with const.
> > I see, but I run into problems while developing this caused by the fact that I could modify UserVF inside the function. Would a const ref be better? I can try that, if it doesn't work and you don't like this const value, I will revert the interface to the original one.
> It should be a coding standard question. It may help against accidental assignment in the implementation to have a by-value const. Unfortunately, this function implementation detail leaks into the function signature.
> 
> Since I don't see this anywhere else in the LLVM code base, so I'd prefer to not do it. Otherwise, if we want to apply this consequently, we'd have to add const to by-value parameters to many functions (e.g. `setBestPlan` below).
> 
> Const ref would be worse.
Thank you for explaining. I have removed the `const` and restored the original interface..


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

https://reviews.llvm.org/D57598





More information about the llvm-commits mailing list