[PATCH] D84324: AMDGPU/GlobalISel: Lower G_FREM

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

In D84324#2169535 <https://reviews.llvm.org/D84324#2169535>, @foad wrote:

> In D84324#2169378 <https://reviews.llvm.org/D84324#2169378>, @arsenm wrote:
>
> > The errors aren't small, and aren't just edge cases:
>
>
> The ones that return +/- inf are because the division overflows. The others look like rounding error in the division when the result of x/y is large but doesn't overflow - this can easily lead to a result with the wrong sign, or with magnitude larger than y. I don't think it's realistic to try to fix these problems with an inline expansion. It really needs a library function.
>
> I suppose the question is: is this patch still a useful default implementation of frem?


For something that doesn't work perfectly, I don't think it belongs in the generic code. It would be more palatable to keep this in AMDGPU to match the DAG behavior


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

https://reviews.llvm.org/D84324