[PATCH] D84324: AMDGPU/GlobalISel: Lower G_FREM

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

In D84324#2170042 <https://reviews.llvm.org/D84324#2170042>, @arsenm wrote:

> 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


Maybe it should also fail to legalize if it's not afn?


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

https://reviews.llvm.org/D84324