[PATCH] D22898: AMDGPU: Fix ffloor for SI

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

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

> In https://reviews.llvm.org/D22898#855778, @arsenm wrote:
>
> > 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.
>
>
> What does that mean?


IEEE canonicalize. http://www.llvm.org/docs/LangRef.html#llvm-canonicalize-intrinsic

NaNs are quieted, denormals may be flushed


https://reviews.llvm.org/D22898