[PATCH] D70226: Add an option to disable strict float node mutating to an normal float node

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2822
+      break;
     if (TLI.getStrictFPOperationAction(Node->getOpcode(),
                                        Node->getValueType(0))
----------------
LiuChen3 wrote:
> uweigand wrote:
> > Sorry, this is still not quite what I expected: now you've removed the second comment, which was actually correct (and necessary) ...
> > 
> > My point is that the two "if" statements implement two **very different** things, the first is a correctness issue, the second is just a performance optimization.  So we really ought to have two **different** comments explaining the two different purposes of those if statements, as I had in my original suggestion.
> > 
> > In your first patch that I commented upon earlier, you had two comments, but both were talking about the performance optimization -- this is wrong for the first if, which is all about correctness.  Now you fixed the comment before the first if to talk about correctness, but you removed the second comment completely, which gives the impression that the second if is also about correctness, which it is not ...
> Thanks for your explanation.  But why this is a performance optimization? I thought this conversion was just to allow the backend to make the correct instruction selection without supporting strict-float. The performance optimization means by the promotion of the legal instruction compared to the converting to statck operation?
> Or I misunderstand something?
OK, so when we get here, the back-end has asked common code to "Expand" the STRICT_FP_ROUND operation.  Common code has three options to do so:
1) Emit a libcall
2) Replace it with a FP_ROUND -- only possible if FP_ROUND is "Legal"
3) Replace it with a stack operation (truncating store followed by load)

If we must enforce strict FP semantics, then only option 1) is allowed, since both options 2) and 3) do not respect that semantics.  That is the correctness property that is enforced by the first "if".

Now, if we do not have to enfore strict FP semantics, then either option 1), 2) or 3) would be allowed.  So in case, we make the decision on the relative efficiency of those options, where we'd usually have 2) the fastest, followed by 3), and then 1) as the slowest.  Since 2) is not always possible, we'd choose 2) when it is available, and 3) otherwise.  This is what the second "if" achieves.

Does this make it clearer?  If you find some other wording for those comments that convey that explanation in a better way, feel free to update them :-)



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70226/new/

https://reviews.llvm.org/D70226