I worked on some patterns to address the code generated by these
examples, which came out of some differential testing against GCC. The patterns
will be in a follow-up patch.
Details
- Reviewers
luismarques
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/test/CodeGen/RISCV/double-convert-indirect.ll | ||
---|---|---|
10 | 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. |
llvm/test/CodeGen/RISCV/double-convert-indirect.ll | ||
---|---|---|
10 | 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? |
llvm/test/CodeGen/RISCV/double-convert-indirect.ll | ||
---|---|---|
10 | 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. |
llvm/test/CodeGen/RISCV/double-convert-indirect.ll | ||
---|---|---|
10 | 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. |
Address @asb's feedback
- Renamed the test
- Added lines for F-extension only runs, showing libcalls.
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.