<html dir="ltr"><head></head><body style="text-align:left; direction:ltr;"><div>On Wed, 2018-12-05 at 11:33 -0600, Jason Ekstrand wrote:</div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Tue, Dec 4, 2018 at 1:17 AM Iago Toral Quiroga <<a href="mailto:itoral@igalia.com">itoral@igalia.com</a>> wrote:<br></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex">---<br>
src/compiler/spirv/vtn_glsl450.c | 36 +++++++++++++++++++++-----------<br>
1 file changed, 24 insertions(+), 12 deletions(-)<br>
<br>
diff --git a/src/compiler/spirv/vtn_glsl450.c b/src/compiler/spirv/vtn_glsl450.c<br>
index 4345c9c61a3..9cda80c5137 100644<br>
--- a/src/compiler/spirv/vtn_glsl450.c<br>
+++ b/src/compiler/spirv/vtn_glsl450.c<br>
@@ -238,8 +238,10 @@ build_fsum(nir_builder *b, nir_ssa_def **xs, int terms)<br>
static nir_ssa_def *<br>
build_atan(nir_builder *b, nir_ssa_def *y_over_x)<br>
{<br>
+ const uint32_t bit_size = y_over_x->bit_size;<br>
+<br>
nir_ssa_def *abs_y_over_x = nir_fabs(b, y_over_x);<br>
- nir_ssa_def *one = nir_imm_float(b, 1.0f);<br>
+ nir_ssa_def *one = nir_imm_floatN_t(b, 1.0f, bit_size);<br>
<br>
/*<br>
* range-reduction, first step:<br>
@@ -265,25 +267,35 @@ build_atan(nir_builder *b, nir_ssa_def *y_over_x)<br>
nir_ssa_def *x_9 = nir_fmul(b, x_7, x_2);<br>
nir_ssa_def *x_11 = nir_fmul(b, x_9, x_2);<br>
<br>
+ const float coef[] = {<br>
+ 0.9999793128310355f,<br>
+ -0.3326756418091246f,<br>
+ 0.1938924977115610f,<br>
+ -0.1173503194786851f,<br>
+ 0.0536813784310406f,<br>
+ -0.0121323213173444f,<br>
+ };<br>
+<br>
nir_ssa_def *polynomial_terms[] = {<br>
- nir_fmul(b, x, nir_imm_float(b, 0.9999793128310355f)),<br>
- nir_fmul(b, x_3, nir_imm_float(b, -0.3326756418091246f)),<br>
- nir_fmul(b, x_5, nir_imm_float(b, 0.1938924977115610f)),<br>
- nir_fmul(b, x_7, nir_imm_float(b, -0.1173503194786851f)),<br>
- nir_fmul(b, x_9, nir_imm_float(b, 0.0536813784310406f)),<br>
- nir_fmul(b, x_11, nir_imm_float(b, -0.0121323213173444f)),<br>
+ nir_fmul(b, x, nir_imm_floatN_t(b, coef[0], bit_size)),<br>
+ nir_fmul(b, x_3, nir_imm_floatN_t(b, coef[1], bit_size)),<br>
+ nir_fmul(b, x_5, nir_imm_floatN_t(b, coef[2], bit_size)),<br>
+ nir_fmul(b, x_7, nir_imm_floatN_t(b, coef[3], bit_size)),<br>
+ nir_fmul(b, x_9, nir_imm_floatN_t(b, coef[4], bit_size)),<br>
+ nir_fmul(b, x_11, nir_imm_floatN_t(b, coef[5], bit_size)),<br></blockquote><div><br></div><div>Any reason why you split the coefficients out into their own array? Just to avoid line wrapping?</div></div></div></blockquote><div><br></div><div>Yes, I think that was the only reason.</div><div><br></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>In a recent commit, I added nir_iadd_imm and nir_imul_imm helpers for multiplying by or adding an immediate. It might be worth doing the same thing for multiplying and add by floats for these kinds of computations.<br></div></div></div></blockquote><div><br></div><div>Right, yes, that is probably a good idea, I'll do that.</div><div><br></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex">
};<br>
<br>
nir_ssa_def *tmp =<br>
build_fsum(b, polynomial_terms, ARRAY_SIZE(polynomial_terms));<br>
<br>
/* range-reduction fixup */<br>
+ nir_ssa_def *minus_2 = nir_imm_floatN_t(b, -2.0f, bit_size);<br>
+ nir_ssa_def *m_pi_2 = nir_imm_floatN_t(b, M_PI_2f, bit_size);<br>
+ nir_ssa_def *b2f = nir_b2f(b, nir_flt(b, one, abs_y_over_x));<br>
+ b2f->bit_size = bit_size; /* do we prefer b2f<bitsize> opcodes? */<br></blockquote><div><br></div><div>I have patches which will fix this at least somewhat. For right now, what you did there is fine.</div></div></div></blockquote><div><br></div><div>Ah yes, I imagine it is likely that your patches will land ahead of this series so I should remember to edit this when that happens.</div><div><br></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex">
tmp = nir_fadd(b, tmp,<br>
- nir_fmul(b,<br>
- nir_b2f(b, nir_flt(b, one, abs_y_over_x)),<br>
- nir_fadd(b, nir_fmul(b, tmp,<br>
- nir_imm_float(b, -2.0f)),<br>
- nir_imm_float(b, M_PI_2f))));<br>
+ nir_fmul(b, b2f,<br>
+ nir_fadd(b, nir_fmul(b, tmp, minus_2), m_pi_2)));<br>
<br>
/* sign fixup */<br>
return nir_fmul(b, tmp, nir_fsign(b, y_over_x));<br>
</blockquote></div></div></blockquote></body></html>