[Mesa-dev] [PATCH mesa] radv: s/abs/fabsf/ for floats

[Matt Turner]

On Thu, Oct 18, 2018 at 1:28 PM Roland Scheidegger <sroland at vmware.com> wrote:
>
> Am 18.10.18 um 19:25 schrieb Matt Turner:
> > On Thu, Oct 18, 2018 at 8:46 AM Eric Engestrom <eric.engestrom at intel.com> wrote:
> >>
> >> Fixes: a4c4efad89eceb26cf82 "radv: Rework guard band calculation"
> >> Cc: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
> >> Signed-off-by: Eric Engestrom <eric.engestrom at intel.com>
> >> ---
> >>  src/amd/vulkan/si_cmd_buffer.c | 8 ++++----
> >>  1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/src/amd/vulkan/si_cmd_buffer.c b/src/amd/vulkan/si_cmd_buffer.c
> >> index de057657ee70d354e910..52daf99414790d4764b6 100644
> >> --- a/src/amd/vulkan/si_cmd_buffer.c
> >> +++ b/src/amd/vulkan/si_cmd_buffer.c
> >> @@ -516,16 +516,16 @@ si_write_scissors(struct radeon_cmdbuf *cs, int first,
> >>                 VkRect2D scissor = si_intersect_scissor(&scissors[i], &viewport_scissor);
> >>
> >>                 get_viewport_xform(viewports + i, scale, translate);
> >> -               scale[0] = abs(scale[0]);
> >> -               scale[1] = abs(scale[1]);
> >> +               scale[0] = fabsf(scale[0]);
> >> +               scale[1] = fabsf(scale[1]);
> >>
> >>                 if (scale[0] < 0.5)
> >
> > You might want to suffix these immediates with f at the same time. As
> > is, this will convert scale[0] to a double before the compairson
> > against 0.5. I'm assuming that scale[0] is a float, which I've
> > inferred from the patch but haven't confirmed.
>
> Not that I don't think doing this isn't good idea, but I think compilers
> are clever enough to figure this out and omit the conversion? At least
> if you don't compile with -O0...

Not in my experience. See commit
23bba717e1178d54927c4968a0466d706a630432 and the preceding patches.

I'm not aware of a reason why the compiler would be required to do the
double conversion, but it seems it does today.