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

[Anna Thomas via Phabricator via llvm-commits]

anna added inline comments.


================
Comment at: llvm/lib/Analysis/IVDescriptors.cpp:814
+     // minimum and maximum intrinsics do not require nsz and nnan flags since
+     // signed zeroes and NaN are these are supported in the intrinsic
+     // implementation.
----------------
dmgreen wrote:
> As far as I understand from the language ref, llvm.vector.reduce.fmin will be unspecified for 0.0/-0.0, and will return a value unless all the inputs are Nan. https://llvm.org/docs/LangRef.html#llvm-vector-reduce-fmin-intrinsic. This matches the behaviour of llvm.minnum. llvm.minimum has stronger semantics for signed zero and propagates nans, so would only be valid to convert with fast math flags.
> 
> I'm not 100% sure that llvm.minnum would need fast math flags, but llvm.minimum would seem to. Does it sound like I have that the right way around?
> As far as I understand from the language ref, llvm.vector.reduce.fmin will be unspecified for 0.0/-0.0, and will return a value unless all the inputs are Nan.
Ah, looks like vector.reduce.fmin was designed with only minnum in mind according to the langref: "This instruction has the same comparison semantics as the ‘llvm.minnum.*’ intrinsic. ".  Adding the flags `nnan` and `nsz` for llvm.minimum so that we can feed it into vector.reduce.fmin unfortunately defeats the purpose of these minimum intrinsic. 
What we need is something like:
```
%v = call <2 x float> @llvm.minimum.v2f32(...)
...
%F = llvm.vector.reduce.fminimum(%v)
```
where llvm.vector.reduce.fminimum follows the semantics of `llvm.minimum`. 
I'm not sure if we can piggyback on the existing llvm.vector.reduce.fmin intrinsic or would need to create a new one. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151482