[PATCH] D12202: Add Support for Small Size Reductions

[Matthew Simpson via llvm-commits]

mssimpso marked 2 inline comments as done.

================
Comment at: include/llvm/Transforms/Utils/LoopUtils.h:197
@@ +196,3 @@
+  /// Returns true if the recurrence kind is an arithmetic kind.
+  static bool isArithmeticRecurrenceKind(RecurrenceKind Kind);
+
----------------
jmolloy wrote:
> What would a non-arithmetic kind be? I'm not sure I understand this.
The intent was to select the recurrence kinds that would be subject to type-promotion (ADD, MUL). We don't need to worry about the bitwise (AND, OR, XOR) or logical (MIN/MAX) operations.

================
Comment at: include/llvm/Transforms/Utils/LoopUtils.h:203
@@ +202,3 @@
+  /// operation. The AND operation is also added to CI.
+  static Instruction *lookThroughAnd(PHINode *Phi, Type *&RT,
+                                     SmallPtrSetImpl<Instruction *> &Visited,
----------------
jmolloy wrote:
> I don't like this interface. But that shouldn't stop this patch I think - I think we need to have a good hard redesign of the interface for RecurrenceDescriptor (and InductionDescriptor which I'm ripping out of LoopVectorize in D12284.
I agree, and I look forward to the re-design. Having said that, I'm happy to improve this interface in the meantime if you have specific concerns.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3336
@@ +3335,3 @@
+      for (unsigned part = 0; part < UF; ++part) {
+        Value *Trunc = Builder.CreateTrunc(RdxExitVal[part], RdxVecTy);
+        Value *Extnd = RdxDesc.isSigned()
----------------
jmolloy wrote:
> Wouldn't it be just as easy to AND with Type->getMask()?
InstCombine isn't smart enough to rewrite the expression in the narrower type if we use an AND. Specifically, InstCombiner::EvaluateInDifferentType() relies on CanEvaluateTruncated() to succeed. So we need to insert the trunc unless we want to rewrite the expression ourselves. One alternative would be to extend InstCombine to handle the AND. Another alternative would be to rewrite the expression in the correct width on the fly as we vectorize. This would avoid the trunc/ext, but I think both alternatives would be more complex to implement. What do you think?


http://reviews.llvm.org/D12202