Adding a small patch to fold the following min/max intrinsics calls-
- fmin(x, fmax(x, y)) -> select isnan(x)?y:x
- fmax(x, fmin(x, y)) -> select isnan(x)?y:x
karthikthecool on Jun 8 2016, 7:45 AM.Authored by
x = NaN
y = 0
fmin(x, fmax(x, y)) -> x ?
Thanks escha for the counter example. I will handle NaN case properly.
Update the patch to handle NaN's.
We eliminate 2 intinsic function call with a compare and select instruction.
Thanks and Regards
Hi David, Matt,
@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".
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.
Thanks again for your review comments. Please let me know your valuable inputs on the comments above.
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 David for the comments. I get your point now. Updated the code to incorporate comment.
Please let me know if you have any other feedback.
I discovered that the libcall already does this nnan compare + select canonicalization (although I don't get the rationale for it, the opposite seems preferable). Are you still working on this patch?