[PATCH] D111077: [LV] Support converting FP add to integer reductions.

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Like I said, I do think a lot of the more common cases of this kind of code would be better suited deleting the loop and transforming to the straight line code. It will be when extra stuff going on in the loop where vectorization will become more useful. It might be worth looking into the loop-deletion case too, at some point.



================
Comment at: llvm/lib/Analysis/IVDescriptors.cpp:235
+  if (Kind == RecurKind::FAddAsInt) {
+    if (!RdxStart->getType()->isFloatTy() && !RdxStart->getType()->isDoubleTy())
+      return false;
----------------
Should half be supported too?


================
Comment at: llvm/lib/Analysis/IVDescriptors.cpp:707
                     I->hasAllowReassoc() ? nullptr : I);
   case Instruction::FSub:
   case Instruction::FAdd:
----------------
Is fsub supported? I don't see any tests or the code to handle them.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:1349
+    Builder.SetInsertPoint(State.CFG.VectorPreHeader->getTerminator());
+    StartV = Builder.CreateFPToSI(StartV, RdxDesc.getRecurrenceType());
+  }
----------------
Are all start values valid? What if it is not an integer? Or if it is already -0x1.0p24? Or not a multiple of the induction amount? (I'm not sure any of those will be wrong, they might be fine and would start to "saturate" at the same point. I would have to test it to see what would happen).


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/fadd-reduction-as-int.ll:27
+; CHECK-NEXT:    [[BIN_RDX:%.*]] = add <4 x i32> [[TMP3]], [[TMP2]]
+; CHECK-NEXT:    [[TMP5:%.*]] = call i32 @llvm.vector.reduce.add.v4i32(<4 x i32> [[BIN_RDX]])
+; CHECK-NEXT:    [[TMP6:%.*]] = sitofp i32 [[TMP5]] to float
----------------
What prevents this add from overflowing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111077