<div dir="ltr">On 23 August 2013 13:19, Matt Turner <span dir="ltr"><<a href="mailto:mattst88@gmail.com" target="_blank">mattst88@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div class=""><div class="h5">On Fri, Aug 23, 2013 at 8:55 AM, Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> wrote:<br>
> On 22 August 2013 16:08, Matt Turner <<a href="mailto:mattst88@gmail.com">mattst88@gmail.com</a>> wrote:<br>
>><br>
>> ---<br>
>>  src/glsl/builtins/ir/<a href="http://frexp.ir" target="_blank">frexp.ir</a>                   | 25<br>
>> +++++++++++++++++++++++++<br>
>>  src/glsl/builtins/ir/<a href="http://ldexp.ir" target="_blank">ldexp.ir</a>                   | 25<br>
>> +++++++++++++++++++++++++<br>
>>  src/glsl/builtins/profiles/ARB_gpu_shader5.glsl | 10 ++++++++++<br>
>>  3 files changed, 60 insertions(+)<br>
>>  create mode 100644 src/glsl/builtins/ir/<a href="http://frexp.ir" target="_blank">frexp.ir</a><br>
>>  create mode 100644 src/glsl/builtins/ir/<a href="http://ldexp.ir" target="_blank">ldexp.ir</a><br>
>><br>
>> diff --git a/src/glsl/builtins/ir/<a href="http://frexp.ir" target="_blank">frexp.ir</a> b/src/glsl/builtins/ir/<a href="http://frexp.ir" target="_blank">frexp.ir</a><br>
>> new file mode 100644<br>
>> index 0000000..a514994<br>
>> --- /dev/null<br>
>> +++ b/src/glsl/builtins/ir/<a href="http://frexp.ir" target="_blank">frexp.ir</a><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 going to<br>
> break assumptions in a lot of our optimization passes.<br>
<br>
</div></div>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>
</blockquote></div><br></div><div class="gmail_extra">Hmm, interesting.<br><br>The way LLVM solves this problem, as I understand it, is through so-called "intrinsic functions" (<a href="http://llvm.org/docs/LangRef.html#intrinsic-functions">http://llvm.org/docs/LangRef.html#intrinsic-functions</a>).  I wonder if we should start doing that in Mesa.<br>
<br></div><div class="gmail_extra">Briefly, here is what it would look like, using uaddCarry as an example:<br><br></div><div class="gmail_extra">1. First we do an inefficient implementation of uaddCarry in terms of existing GLSL functions, much like you did for frexp in your frexp_to_arith lowering pass, except that we do it in src/glsl/builtins/glsl/uaddCarry.glsl, so it's a little easier to review :).  Optimization passes already deal with function "out" parameters properly, and function inlining automatically splices in the proper code during linking.<br>
<br></div><div class="gmail_extra">2. For back-ends that don't have an efficient native way to do uaddCarry, we're done.  The uaddCarry function works as is.<br></div><div class="gmail_extra"><br></div><div class="gmail_extra">
3. For back-ends that do have an efficient way to do uaddCarry, we add a mechanism to allow the back-end to tell the linker: "don't inline the definition of this built-in.  Just leave it as an ir_call because I have my own special implementation of it"*.<br>
<br></div><div class="gmail_extra">4. In the back-end visitor code, the ir_call visitor looks at the name of the function being called.  If it's "uaddCarry", then the back-end visitor just emits the efficient back-end code.  Any other ir_calls should have been eliminated by the function inlining.<br>
</div><div class="gmail_extra"><br></div><div class="gmail_extra">*We'll need to be careful to make sure that the right thing happens if the user overrides uaddCarry with their own user-defined function, of course :)<br>
<br><br></div><div class="gmail_extra">Now that I've actually thought through it, I'm really excited about this idea.  It seems way more straightforward than what we are currently doing (e.g. in lower_packing_builtins.cpp), and it works nicely with the other back-ends because if a back-end doesn't advertise an intrinsic definition of a given function, it automtically gets the version declared in src/glsl/builtins without having to do any extra work.<br>
</div><div class="gmail_extra"><br>What do you think?<br></div></div>