[Mesa-dev] [PATCH 05/15] glsl: Add new {fr, ld}exp built-ins IR and prototypes.

[Matt Turner]

On Mon, Aug 26, 2013 at 5:49 PM, Ian Romanick <idr at freedesktop.org> wrote:
> On 08/23/2013 02:02 PM, Paul Berry wrote:
>>
>> On 23 August 2013 13:19, Matt Turner <mattst88 at gmail.com
>> <mailto:mattst88 at gmail.com>> wrote:
>>
>>     On Fri, Aug 23, 2013 at 8:55 AM, Paul Berry <stereotype441 at gmail.com
>>     <mailto:stereotype441 at gmail.com>> wrote:
>>      > On 22 August 2013 16:08, Matt Turner <mattst88 at gmail.com
>>     <mailto:mattst88 at gmail.com>> wrote:
>>      >>
>>      >> ---
>>      >>  src/glsl/builtins/ir/frexp.ir <http://frexp.ir>
>>        | 25
>>      >> +++++++++++++++++++++++++
>>      >>  src/glsl/builtins/ir/ldexp.ir <http://ldexp.ir>
>>
>>        | 25
>>      >> +++++++++++++++++++++++++
>>      >>  src/glsl/builtins/profiles/ARB_gpu_shader5.glsl | 10 ++++++++++
>>      >>  3 files changed, 60 insertions(+)
>>      >>  create mode 100644 src/glsl/builtins/ir/frexp.ir
>> <http://frexp.ir>
>>      >>  create mode 100644 src/glsl/builtins/ir/ldexp.ir
>> <http://ldexp.ir>
>>      >>
>>      >> diff --git a/src/glsl/builtins/ir/frexp.ir <http://frexp.ir>
>>     b/src/glsl/builtins/ir/frexp.ir <http://frexp.ir>
>>
>>      >> new file mode 100644
>>      >> index 0000000..a514994
>>      >> --- /dev/null
>>      >> +++ b/src/glsl/builtins/ir/frexp.ir <http://frexp.ir>
>>
>>      >> @@ -0,0 +1,25 @@
>>      >> +((function frexp
>>      >> +   (signature float
>>      >> +     (parameters
>>      >> +       (declare (in) float x)
>>      >> +       (declare (out) int exp))
>>      >> +     ((return (expression float frexp (var_ref x) (var_ref
>> exp)))))
>>      >
>>      >
>>      > Having an ir_expression that writes to one of its parameters is
>>     going to
>>      > break assumptions in a lot of our optimization passes.
>>
>>     I'm concerned that that may be a problem we have to solve anyway.
>>
>>     While our hardware doesn't support an frexp instruction (like e.g.,
>>     AMD does) and we could probably do what you suggest, we do have
>>     instructions that correspond directly to the uaddCarry() and
>>     usubBorrow() built-ins in this same extension. They return a value and
>>     also have an out parameter.
>>
>>     genUType uaddCarry(genUType x, genUType y, out genUType carry);
>>     genUType usubBorrow(genUType x, genUType y, out genUType borrow);
>>
>>     We could probably avoid the problem you describe by lowering them, but
>>     it's feeling increasingly distasteful.
>>
>>     Your code would make a good piglit test. I'll do some experiments.
>>
>>
>> Hmm, interesting.
>>
>> The way LLVM solves this problem, as I understand it, is through
>> so-called "intrinsic functions"
>> (http://llvm.org/docs/LangRef.html#intrinsic-functions).  I wonder if we
>> should start doing that in Mesa.
>>
>> Briefly, here is what it would look like, using uaddCarry as an example:
>>
>> 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.
>>
>> 2. For back-ends that don't have an efficient native way to do
>> uaddCarry, we're done.  The uaddCarry function works as is.
>>
>> 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"*.
>
>
> I had thought about solving this in a slightly different way, but there are
> a couple potential tricky bits.
>
> Provide an implementation of the built-in function in the GLSL library.
>
>     float frexp(float x, out int exponent)
>     {
>         return __intrinsic_frexp(x, exponent);
>     }
>
> Provide a default implementation of the intrinsic elsewhere.
>
> Allow drivers to supply an alternate library with custom versions of the
> intrinsics.
>
> Since the GLSL library's frexp is the same in either case, the problem Paul
> identifies below should be avoided.
>
> 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.

Maybe this is a problem in the general case, but I think the only
thing we'd want to use intrinsics for at the moment are exactly the
things you can't consider to be constant expressions -- because of the
multiple outputs.