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

Ulrich Weigand via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 20 04:25:47 PST 2019


uweigand added inline comments.


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


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

https://reviews.llvm.org/D70226





More information about the llvm-commits mailing list