[PATCH] D21137: Instcombile min/max intrinsics calls

[Karthik Bhat via llvm-commits]

karthikthecool added a subscriber: Matt.
karthikthecool added a comment.

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.

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

Regards
Karthik


http://reviews.llvm.org/D21137