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

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D28508#660740, @jlebar wrote:

> > This seems to indicate that, if I'm reading the code correctly, we have a bug here for other backends for sqrt() when no extra iterations are requested (it will directly return the rsqrt estimate instead of the proper sqrt estimate)
>
> Wow, what an amazingly broken api.
>
> How's this for a plan: Let me implement this function correctly for NVPTX and check in a test.  Then I will at the very least improve the comment on the virtual function and try to contact the backend owners.  I would normally volunteer to refactor the API, but it seems that nobody is testing it, otherwise they would have realized that it is broken, so I'm pretty hesitant to go after it myself.


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.


https://reviews.llvm.org/D28508