[PATCH] D104237: [RISCV][VP] Lower FP VP ISD nodes to RVV instructions

Wed Jun 16 23:19:10 PDT 2021

rogfer01 accepted this revision.
rogfer01 added a comment.
This revision is now accepted and ready to land.

In D104237#2822207 <https://reviews.llvm.org/D104237#2822207>, @frasercrmck wrote:

> In D104237#2821512 <https://reviews.llvm.org/D104237#2821512>, @frasercrmck wrote:
>
>> In D104237#2819994 <https://reviews.llvm.org/D104237#2819994>, @craig.topper wrote:
>>
>>> Many targets scalarize frem on fixed vectors. It looks like frem on scalable vectors currently crashes on SVE because the type legalizer can't unroll it.
>>
>> Indeed. I don't think we or any target support any level of scalable-vector `frem` right now, so I think this discussion is orthogonal to this VP patch. But I'll spend some time today seeing if there's anything we can do since it's come up.
>
> I spent a bit of time looking at `musl`'s implementation of `fmod` and running it through Codeplay's whole-function vectorizer. I get vectorized output and it's passing `musl`'s own tests for rounding, NaNs, etc. (not exceptions but I don't think that's a problem for LLVM's `fmod` instruction), but it's definitely not something we want to emit inline with `IRBuilder` or with `SelectionDAG`: control flow, loops, you name it. I think it'd have to be lowered to a vector library call, realistically-speaking. That or we arrive back at the beginning of this discussion and emit a loop with the scalar `fmod` and it expands itself to a libcall.

Thanks for looking into it. In our downstream we defer it too to a function call so we can complete the compilation and elsewhere we can provide an implementation (there is a number of other ISD nodes that we already lower this way like `exp`, `sin`, `cos`, etc.). AFAICT Arm SVE is not lowering any operation to `LibCall`yet so perhaps this conversation should be raised in the mailing list. Not sure if we need to align `compiler-rt`/`libgcc` in terms of naming conventions here.

I'm happy to land this without `frem`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104237