[PATCH] D67148: [LoopVectorize][PowerPC] Estimate int and float register pressure separately in loop-vectorize

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

Thanks for exploring this direction.



================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:798
+  /// The ID values of generic scalar and vector register class.
+  enum GenericRegisterClass {
+    GenericIntScalarRC = 1,
----------------
I'm not sure that these defaults make sense. Many targets won't even have these as distinct classes (e.g., PowerPC with VSX). I think that we should have the default implementation just return one register class, 0, with its current default (which I suppose is 8 registers), and the default implementation will put everything in that one class. Then, I don't think that we need this enum at all.

My impression is that you decided to do it this way so that you could write in the other targets:

  bool Vector = (ClassID == TargetTransformInfo::GenericVectorRC);

but I think it's better to just give all of the other targets which did something with Vector two register classes, and return the second one for all types which are vector types. That should match the current behavior and then the targets can customize as they see fit. But I'd leave this all within each target (there's no need to expose generic classes because there's no need for a generic meaning).


================
Comment at: llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp:1390
+  // TODO: Need distinguish register class?
+  unsigned TTIRegNum = TTI->getNumberOfRegisters(TargetTransformInfo::GenericIntScalarRC) - 1;
   if (C.NumRegs > TTIRegNum) {
----------------
This can be:

  unsigned TTIRegNum = TTI->getNumberOfRegisters(TTI->getRegisterClassForType(F.getType(), false)) - 1;




================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7768
   // interleaving.
-  if (!TTI->getNumberOfRegisters(true) && TTI->getMaxInterleaveFactor(1) < 2)
+  if (!TTI->getNumberOfRegisters(TargetTransformInfo::GenericVectorRC) && TTI->getMaxInterleaveFactor(1) < 2)
     return false;
----------------
I think that we can just make a separate function for this:

  TTI->hasVectorRegisters()

(and then use that here and in the SLP vectorizer).


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

https://reviews.llvm.org/D67148