<div dir="ltr">On 26 August 2013 17:49, Ian Romanick <span dir="ltr"><<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On 08/23/2013 02:02 PM, Paul Berry wrote:<br>
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
On 23 August 2013 13:19, Matt Turner <<a href="mailto:mattst88@gmail.com" target="_blank">mattst88@gmail.com</a><br></div><div class="im">
<mailto:<a href="mailto:mattst88@gmail.com" target="_blank">mattst88@gmail.com</a>>> wrote:<br>
<br>
On Fri, Aug 23, 2013 at 8:55 AM, Paul Berry <<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a><br></div><div class="im">
<mailto:<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.<u></u>com</a>>> wrote:<br>
> On 22 August 2013 16:08, Matt Turner <<a href="mailto:mattst88@gmail.com" target="_blank">mattst88@gmail.com</a><br></div>
<mailto:<a href="mailto:mattst88@gmail.com" target="_blank">mattst88@gmail.com</a>>> wrote:<br>
>><br>
>> ---<br>
>> src/glsl/builtins/ir/<a href="http://frexp.ir" target="_blank">frexp.ir</a> <<a href="http://frexp.ir" target="_blank">http://frexp.ir</a>><br>
| 25<br>
>> +++++++++++++++++++++++++<br>
>> src/glsl/builtins/ir/<a href="http://ldexp.ir" target="_blank">ldexp.ir</a> <<a href="http://ldexp.ir" target="_blank">http://ldexp.ir</a>><div class="im"><br>
| 25<br>
>> +++++++++++++++++++++++++<br>
>> src/glsl/builtins/profiles/<u></u>ARB_gpu_shader5.glsl | 10 ++++++++++<br>
>> 3 files changed, 60 insertions(+)<br></div>
>> create mode 100644 src/glsl/builtins/ir/<a href="http://frexp.ir" target="_blank">frexp.ir</a> <<a href="http://frexp.ir" target="_blank">http://frexp.ir</a>><br>
>> create mode 100644 src/glsl/builtins/ir/<a href="http://ldexp.ir" target="_blank">ldexp.ir</a> <<a href="http://ldexp.ir" target="_blank">http://ldexp.ir</a>><br>
>><br>
>> diff --git a/src/glsl/builtins/ir/<a href="http://frexp.ir" target="_blank">frexp.<u></u>ir</a> <<a href="http://frexp.ir" target="_blank">http://frexp.ir</a>><br>
b/src/glsl/builtins/ir/<a href="http://frexp.ir" target="_blank">frexp.<u></u>ir</a> <<a href="http://frexp.ir" target="_blank">http://frexp.ir</a>><div class="im"><br>
>> new file mode 100644<br>
>> index 0000000..a514994<br>
>> --- /dev/null<br></div>
>> +++ b/src/glsl/builtins/ir/<a href="http://frexp.ir" target="_blank">frexp.<u></u>ir</a> <<a href="http://frexp.ir" target="_blank">http://frexp.ir</a>><div><div class="h5"><br>
>> @@ -0,0 +1,25 @@<br>
>> +((function frexp<br>
>> + (signature float<br>
>> + (parameters<br>
>> + (declare (in) float x)<br>
>> + (declare (out) int exp))<br>
>> + ((return (expression float frexp (var_ref x) (var_ref exp)))))<br>
><br>
><br>
> Having an ir_expression that writes to one of its parameters is<br>
going to<br>
> break assumptions in a lot of our optimization passes.<br>
<br>
I'm concerned that that may be a problem we have to solve anyway.<br>
<br>
While our hardware doesn't support an frexp instruction (like e.g.,<br>
AMD does) and we could probably do what you suggest, we do have<br>
instructions that correspond directly to the uaddCarry() and<br>
usubBorrow() built-ins in this same extension. They return a value and<br>
also have an out parameter.<br>
<br>
genUType uaddCarry(genUType x, genUType y, out genUType carry);<br>
genUType usubBorrow(genUType x, genUType y, out genUType borrow);<br>
<br>
We could probably avoid the problem you describe by lowering them, but<br>
it's feeling increasingly distasteful.<br>
<br>
Your code would make a good piglit test. I'll do some experiments.<br>
<br>
<br>
Hmm, interesting.<br>
<br>
The way LLVM solves this problem, as I understand it, is through<br>
so-called "intrinsic functions"<br>
(<a href="http://llvm.org/docs/LangRef.html#intrinsic-functions" target="_blank">http://llvm.org/docs/LangRef.<u></u>html#intrinsic-functions</a>). I wonder if we<br>
should start doing that in Mesa.<br>
<br>
Briefly, here is what it would look like, using uaddCarry as an example:<br>
<br>
1. First we do an inefficient implementation of uaddCarry in terms of<br>
existing GLSL functions, much like you did for frexp in your<br>
frexp_to_arith lowering pass, except that we do it in<br>
src/glsl/builtins/glsl/<u></u>uaddCarry.glsl, so it's a little easier to review<br>
:). Optimization passes already deal with function "out" parameters<br>
properly, and function inlining automatically splices in the proper code<br>
during linking.<br>
<br>
2. For back-ends that don't have an efficient native way to do<br>
uaddCarry, we're done. The uaddCarry function works as is.<br>
<br>
3. For back-ends that do have an efficient way to do uaddCarry, we add a<br>
mechanism to allow the back-end to tell the linker: "don't inline the<br>
definition of this built-in. Just leave it as an ir_call because I have<br>
my own special implementation of it"*.<br>
</div></div></blockquote>
<br>
I had thought about solving this in a slightly different way, but there are a couple potential tricky bits.<br>
<br>
Provide an implementation of the built-in function in the GLSL library.<br>
<br>
float frexp(float x, out int exponent)<br>
{<br>
return __intrinsic_frexp(x, exponent);<br>
}<br>
<br>
Provide a default implementation of the intrinsic elsewhere.<br>
<br>
Allow drivers to supply an alternate library with custom versions of the intrinsics.<br>
<br>
Since the GLSL library's frexp is the same in either case, the problem Paul identifies below should be avoided.<br>
<br>
The tricky bit, and the problem we always come to when talking about intrinsics is dealing with constant expressions. That doesn't (shouldn't?) apply to this case because of the out parameter, but it may apply to other cases.<br>
</blockquote><div><br></div><div>Yeah, good point about constant expressions. With my proposal, that could be addressed by having the constant expression evaluator always recurse into the GLSL implementation, regardless of whether the function is an intrinsic (this should be fine, since the only reason for the intrinsic version of the function to be used is to take advantage of efficient instructions in the GPU).<br>
<br></div><div>I confess that I don't understand the rest of your proposal as well as I would like. Maybe the three of us should discuss it in person next time we're in the office.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Right now an application could do:<br>
<br>
float foo[packUnorm2x16(vec2(1,0))];<br>
<br>
If packUnorm2x16 becomes __intrinsic_packUnorm2x16, the constant expression evaluator has to be able to handle whatever __intrinsic_packUnorm2x16 becomes.<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
4. In the back-end visitor code, the ir_call visitor looks at the name<br>
of the function being called. If it's "uaddCarry", then the back-end<br>
visitor just emits the efficient back-end code. Any other ir_calls<br>
should have been eliminated by the function inlining.<br>
<br>
*We'll need to be careful to make sure that the right thing happens if<br>
the user overrides uaddCarry with their own user-defined function, of<br>
course :)<br>
<br>
<br>
Now that I've actually thought through it, I'm really excited about this<br>
idea. It seems way more straightforward than what we are currently<br>
doing (e.g. in lower_packing_builtins.cpp), and it works nicely with the<br>
other back-ends because if a back-end doesn't advertise an intrinsic<br>
definition of a given function, it automtically gets the version<br>
declared in src/glsl/builtins without having to do any extra work.<br>
<br>
What do you think?<br>
<br>
<br></div><div class="im">
______________________________<u></u>_________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/<u></u>mailman/listinfo/mesa-dev</a><br>
<br>
</div></blockquote>
<br>
</blockquote></div><br></div></div>