[PATCH] D151482: [LV] Add support for minimum/maximum intrinsics

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Thanks. I think this LGTM, with some Nits



================
Comment at: llvm/include/llvm/Analysis/IVDescriptors.h:51
+  FMinimum,   ///< FP min with llvm.minimum semantics
+  FMaximum,   ///< FP max with llvm.maximum semantics 
   FMulAdd,    ///< Fused multiply-add of floats (a * b + c).
----------------
Nit: Remove extra whitespace


================
Comment at: llvm/lib/Transforms/Utils/LoopUtils.cpp:936
+  case RecurKind::FMinimum:
+    return CmpInst::FCMP_OLT;
+  case RecurKind::FMaximum:
----------------
If this is unused, could it be removed? I think it goes via getMinMaxReductionIntrinsicOp?

I'm not sure if the expansion to cmp+select would be valid without nnan and nsz fmf flags, creating the intrinsic like you have it sounds much better.


================
Comment at: llvm/test/Transforms/LoopVectorize/minmax_reduction.ll:1
-; RUN: opt -S -passes=loop-vectorize,dce -force-vector-width=2 -force-vector-interleave=1  < %s | FileCheck %s
+  ; RUN: opt -S -passes=loop-vectorize,dce -force-vector-width=2 -force-vector-interleave=2  < %s | FileCheck %s
 
----------------
Nit: Whitespace


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151482