[PATCH] D22898: AMDGPU: Fix ffloor for SI

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 29 13:04:28 PDT 2017


arsenm added a comment.

In https://reviews.llvm.org/D22898#501973, @mareko wrote:

> In https://reviews.llvm.org/D22898#501345, @arsenm wrote:
>
> > In https://reviews.llvm.org/D22898#501301, @arsenm wrote:
> >
> > > In https://reviews.llvm.org/D22898#499043, @nhaehnle wrote:
> > >
> > > > Is the MIN needed for correctness at all? Looking at the workaround docs, I see the explanation that "[FRACT] is outputting 1.0 for very small negative inputs). Sounds to me like v_fract is correctly in the range [0, 1.0), except for those very small negative inputs, where it returns 1.0 (which happens to be correct for the ffloor lowering).
> > >
> > >
> > > I guess so? I don't know the details of the bug but this passes conformance now
> >
> >
> > Thinking about it more this makes sense. 1.0 will skip the fract at exactly 1.0. up to 0.99... fract is used
>
>
> How does it make any sense? fract should return values in [0, 1). SI has a bug that it returns 1 incorrectly in one case. Doing min(x, 1) will have no effect on the result of buggy fract. That min() is a no-op operation.


It's not actually a no-op, it's a canonicalize.


https://reviews.llvm.org/D22898





More information about the llvm-commits mailing list