[PATCH] D53235: [RISCV] Add RV64F codegen support

[Alex Bradbury via Phabricator via llvm-commits]

asb added a comment.

Submitting some old in-line comments that were accidentally left unsubmitted.

Patch still applies cleanly.



================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:8959
+  // fold anyext where ToVT==FromVT
+  if (VT == N0->getValueType(N0.getResNo()))
+    return N0;
----------------
efriedma wrote:
> Not sure how you're ending up in a situation where this matters; getNode() will normally fold away an ANY_EXTEND where the operand and result have the same type.
This fold is insufficient anyway. The issue is that anyextend is left hanging around after one of its children is custom legalised. Just handling everything in a target dag combine avoids this and other problems.


================
Comment at: lib/Target/RISCV/RISCVISelDAGToDAG.cpp:161
+  }
+  case ISD::AssertSext: {
+    if (!Subtarget->is64Bit())
----------------
efriedma wrote:
> The point of this is to try to generate a "narrower" FP_TO_SINT where possible?  Does this transform actually improve performance?
The alternative is fcvt.l.s + sign-extension of the result. Just emitting fcvt.w.s is fewer instructions.


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

https://reviews.llvm.org/D53235