[PATCH] D22898: AMDGPU: Fix ffloor for SI

[Matt Arsenault via llvm-commits]

arsenm added a comment.

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

> In https://reviews.llvm.org/D22898#514040, @arsenm wrote:
>
> > In https://reviews.llvm.org/D22898#501974, @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.
> >
> >
> > Yes, by clamping to exactly 1 it skips the broken 1 value. 0.999999... needs to be passed through fract
>
>
> For incorrect x=fract(y) -> x=1, min(x, 1) -> min(1, 1) -> 1. You need this instead: min(x, 0.9999999........). Or am I missing something?


1 is the incorrect value. 0.99999999999999999 should be correctly handled. By clamping to 0.99999999999999999, you miss correctly handling that one value.


https://reviews.llvm.org/D22898