[PATCH] D21137: Instcombile min/max intrinsics calls

[David Majnemer via llvm-commits]

majnemer added a comment.

In http://reviews.llvm.org/D21137#454408, @karthikthecool wrote:

> Hi David, Matt,
>  Thanks a lot for the review comments. Please find my comments below-
>
> @Matt: I have already added test cases for non nan cases which are ( maxnum_x_y_minnum_y, innum_x_maxnum_x_y. minnum_x_y_maxnum_y, maxnum_x_minnum_x_y) respectively. I feel these should cover all the 4 cases for which patch was added. Please let me know if more test cases are required.
>
> @David: fmin/fmax already handle nan when we can deduce a parameter as "nan at compile time". 
>  If we were to add check at runtime the code will look something like -
>
>   fmin(x,y) -> if(isnan(x|y)) ? ( if(isnan(x)) ? y : x) : fmin(x,y)
>
> for non nan cases which i believe will be mostly called we will have one extra compare and select instruction in addition to the normal fmin call.
>  I feel it may not be worth adding this runtime check. But please let me know if you feel otherwise. I can update the code accordingly.


I am saying that there are situations where we know a-priori that the fmax/fmin call will observe a nan due to annotations on the call that _cannot_ be deduced through other data flow analysis.  I would just check that the call is marked nnan and choose not to emit a select in those situations.

> Thanks again for your review comments. Please let me know your valuable inputs on the comments above.

> 

> Regards

> Karthik





http://reviews.llvm.org/D21137