[PATCH] Refactor identification of reductions and expose them as utility functions

[Renato Golin]

================
Comment at: include/llvm/Transforms/Utils/LoopUtils.h:45-62
@@ -44,1 +44,20 @@
 
+/// This enum represents the kinds of reductions that we support.
+enum ReductionKind {
+  RK_NoReduction,   ///< Not a reduction.
+  RK_IntegerAdd,    ///< Sum of integers.
+  RK_IntegerMult,   ///< Product of integers.
+  RK_IntegerOr,     ///< Bitwise or logical OR of numbers.
+  RK_IntegerAnd,    ///< Bitwise or logical AND of numbers.
+  RK_IntegerXor,    ///< Bitwise or logical XOR of numbers.
+  RK_IntegerMinMax, ///< Min/max implemented in terms of select(cmp()).
+  RK_FloatAdd,      ///< Sum of floats.
+  RK_FloatMult,     ///< Product of floats.
+  RK_FloatMinMax    ///< Min/max implemented in terms of select(cmp()).
+};
+
+// This enum represents the kind of minmax reduction.
+enum MinMaxReductionKind {
+  MRK_Invalid,
+  MRK_UIntMin,
+  MRK_UIntMax,
----------------
anemet wrote:
> I haven't dealt with this part of the vectorizer yet so I only have general comments about some interface improvements, now, that these interfaces are made public.
> 
> Are these enums local to the structures that follow right after?  If yes, they should be local to those structs.
Almost, but I think we can make it be. The only other function that uses it is createMinMaxOp() down there, but it could also be part of this structure/class. Then, moving this enum in there would be trivial. Same goes for ReductionKind and getReductionBinOp().

Internally, using the struct as context on external functions would also work.

================
Comment at: include/llvm/Transforms/Utils/LoopUtils.h:123-126
@@ +122,6 @@
+
+bool AddReductionVar(PHINode *Phi, ReductionKind Kind, Loop *TheLoop,
+                     bool HasFunNoNaNAttr, ReductionDescriptor &RedDes);
+
+bool isReductionPHI(PHINode *Phi, Loop *TheLoop, ReductionDescriptor &RedDes);
+
----------------
anemet wrote:
> These are missing comments.  Also if they operate on a ReductionDesc, they should probably made a member function.  And possibly turning the struct into classes to describe the public interface of the class.
If there are no private members, there isn't much point in making it into a class. But I agree, missing comments.

================
Comment at: include/llvm/Transforms/Utils/LoopUtils.h:128-130
@@ +127,5 @@
+
+bool hasMultipleUsesOf(Instruction *I, SmallPtrSetImpl<Instruction *> &Insts);
+
+bool areAllUsesIn(Instruction *I, SmallPtrSetImpl<Instruction *> &Set);
+
----------------
anemet wrote:
> These don't look like loop-related utilities.  Are you using these in your pass?
> 
> In that case you either want to move them to Instruction.h or make them static member of the above classes if these are specific to the algorithm finding reductions.
These were local functions to the loop vectorizer and loop interchange, and I agree they're very generic. Instruction.h seems like a better place.

http://reviews.llvm.org/D9046

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/