[PATCH] D46130: [NVPTX] Turn on Loop/SLP vectorization

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.


================
Comment at: lib/Target/NVPTX/NVPTXTargetTransformInfo.h:53
+  // NVPTX has infinite registers of all kinds.
+  unsigned getNumberOfRegisters(bool Vector) const { return 1; }
+
----------------
jlebar wrote:
> bkramer wrote:
> > jlebar wrote:
> > > bkramer wrote:
> > > > jlebar wrote:
> > > > > Does 1 have specific meaning?  I don't see this in any of the comments, and that would be a pretty weird API...  (Like, did you mean -1?)
> > > > 1 is the default of the generic implementation, I just copied that and removed the check for vector. I'm not even sure if anyone ever checks the value or just compares it against zero.
> > > I see a few places using the actual value returned: LoopStrengthReduce, LoopVectorize...
> > Right, LoopStrengthReduce calls it with (false), which was returning '1' before and after :)
> > 
> > I'm not sure about LoopVectorize, but conservatively limiting its vectorization factor is probably a good thing.
> OK, I think I see what you're after: You want to make the minimal/safest change that gets vectorization on your testcases?
> 
> That sounds good to me, but can we make the comment on "return 1" express this motivation?
LoopVectorize is using this number to control the amount of interleaving/unrolling it does. The idea being that inverleaving is beneficial until you create too much register pressure (i.e., until you start spilling).

I don't think that setting this to 1 is a good idea. Perfectly reasonable changes to the loop vectorizer in the future could cause this to completely disable vectorizer. You should set this, I suspect, to the maximum number of registers you can have per thread at full occupancy.



Repository:
  rL LLVM

https://reviews.llvm.org/D46130