[PATCH] D84324: AMDGPU/GlobalISel: Lower G_FREM

Fri Jul 24 02:00:28 PDT 2020

foad 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

Sounds reasonable.

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

https://reviews.llvm.org/D84324