[PATCH] D78905: [RISCV][NFC] Tests for indirect float conversion

Sam Elliott via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 30 05:16:48 PDT 2020


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


================
Comment at: llvm/test/CodeGen/RISCV/double-convert-indirect.ll:10
+;;
+;; TODO: Unfortunately, though this only uses 32-bit FP instructions, we cannot
+;; do this optimisation without the D extension as we need 64-bit FP values to
----------------
asb wrote:
> lenary wrote:
> > asb wrote:
> > > If I'm interpreting this comment correctly, you're neglecting to test rv32if/rv64if because your planned optimisation won't improve things (but feel in principle it should be possible to optimise)? If so, I think it would be better to include the rv32if/rv64if cases here too, even if we don't have a solution on the horizon.
> > You are interpreting correctly.
> > 
> > It might be possible to optimise, but not with normal SelectionDAG patterns, because on RV32IF the legaliser has already turned the two operations we want to match on into libcalls.
> > 
> > I can look at doing a follow-up DAGCombiner patch (which would run before the legaliser), but it's not clear to me that this is valid on every architecture, because I'm not super familiar with floats. I can post a draft DAGCombiner patch and seek feedback?
> A DAGCombiner patch might be interesting, but actually I was just suggesting that this test case contain check lines for RV32IF and RV64IF so that we'll be sure to pick it up if some other change means codegen for those cases improves too.
Ok, sure. I'll add those lines and also rename it so it's clear it's for all FP extensions, not just the D extension.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78905





More information about the llvm-commits mailing list