[PATCH] D28508: [NVPTX] Implement NVPTXTargetLowering::getSqrtEstimate.

[Justin Lebar via Phabricator via llvm-commits]

jlebar added a comment.

In https://reviews.llvm.org/D28508#661259, @hfinkel wrote:

> The most natural way I can think of to fix this API, is the make it always expect a rsqrt estimate (including in the ExtraIterations == 0 case). In that case, if a target has a direct sqrt estimate to use in the ExtraIterations == 0 case, it would return SDValue() and then match the sqrt node in TableGen (or in C++). That fix is incompatible with the code here, so I'd like your opinion on this.


I wonder if part of the problem is that the API is just too flexible.  In particular, what if we didn't let you modify NumIters within this function?

We would have three or four TLI functions:

- "get me the default number of Newton's method iters for an (r)sqrt approximation" (could be one method with a bool param or two; I lean towards two because jlebar.com/2011/12/16/Boolean_parameters_to_API_functions_considered_harmful..html)
- "get me an approximate rsqrt"
- "get me an approximate sqrt", where the default returns x * approx_rsqrt(x).

The problem right now is that because the function may modify NumIters, everything has to be stuffed into one big, confusing function.

We don't strictly need "get me an approx sqrt", but defining it this way lets us have a reasonable default, which seems nice.

(I would still rather check in this patch and refactor separately, though.)


https://reviews.llvm.org/D28508