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

LiuChen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 20 07:28:16 PST 2019


LiuChen3 marked an inline comment as done.
LiuChen3 added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2822
+      break;
     if (TLI.getStrictFPOperationAction(Node->getOpcode(),
                                        Node->getValueType(0))
----------------
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?


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

https://reviews.llvm.org/D70226





More information about the llvm-commits mailing list