[Mesa-dev] [PATCH 7.5/11] glsl: Kill __intrinsic_atomic_sub

[Ian Romanick]

On 07/07/2016 04:58 PM, Ilia Mirkin wrote:
> On Thu, Jul 7, 2016 at 5:02 PM, Ian Romanick <idr at freedesktop.org> wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> Just generate an __intrinsic_atomic_add with a negated parameter.
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> ---
>>  src/compiler/glsl/builtin_functions.cpp    | 50 +++++++++++++++++++++++++++---
>>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp |  8 -----
>>  2 files changed, 46 insertions(+), 12 deletions(-)
>>
>> diff --git a/src/compiler/glsl/builtin_functions.cpp b/src/compiler/glsl/builtin_functions.cpp
>> index 941ea12..ef3b2b0 100644
>> --- a/src/compiler/glsl/builtin_functions.cpp
>> +++ b/src/compiler/glsl/builtin_functions.cpp
>> @@ -3310,13 +3310,29 @@ builtin_builder::asin_expr(ir_variable *x, float p0, float p1)
>>                                            mul(abs(x), imm(p1))))))))));
>>  }
>>
>> +/**
>> + * Generate a ir_call to a function with a set of parameters
>> + *
>> + * The input \c params can either be a list of \c ir_variable or a list of
>> + * \c ir_dereference_variable.  In the latter case, all nodes will be removed
>> + * from \c params and used directly as the parameters to the generated
>> + * \c ir_call.
>> + */
>>  ir_call *
>>  builtin_builder::call(ir_function *f, ir_variable *ret, exec_list params)
>>  {
>>     exec_list actual_params;
>>
>> -   foreach_in_list(ir_variable, var, &params) {
>> -      actual_params.push_tail(var_ref(var));
>> +   foreach_in_list_safe(ir_instruction, ir, &params) {
>> +      ir_dereference_variable *d = ir->as_dereference_variable();
>> +      if (d != NULL) {
>> +         d->remove();
>> +         actual_params.push_tail(d);
>> +      } else {
>> +         ir_variable *var = ir->as_variable();
>> +         assert(var != NULL);
>> +         actual_params.push_tail(var_ref(var));
>> +      }
>>     }
>>
>>     ir_function_signature *sig =
>> @@ -5292,8 +5308,34 @@ builtin_builder::_atomic_counter_op1(const char *intrinsic,
>>     MAKE_SIG(glsl_type::uint_type, avail, 2, counter, data);
>>
>>     ir_variable *retval = body.make_temp(glsl_type::uint_type, "atomic_retval");
>> -   body.emit(call(shader->symbols->get_function(intrinsic), retval,
>> -                  sig->parameters));
>> +
>> +   /* Instead of generating an __intrinsic_atomic_sub, generate an
>> +    * __intrinsic_atomic_add with the data parameter negated.
>> +    */
>> +   if (strcmp("__intrinsic_atomic_sub", intrinsic) == 0) {
>> +      ir_variable *const neg_data =
>> +         body.make_temp(glsl_type::uint_type, "neg_data");
>> +
>> +      body.emit(assign(neg_data, neg(data)));
>> +
>> +      exec_list parameters;
>> +
>> +      parameters.push_tail(new(mem_ctx) ir_dereference_variable(counter));
>> +      parameters.push_tail(new(mem_ctx) ir_dereference_variable(neg_data));
> 
> I don't get it ... why change call() to allow taking dereferences and
> create them here rather than just feeding in the ir_variables
> directly?

Oh, I already went down that path.  :)  neg_data would have to be in two
lists at the same time:  the instruction stream and parameters.
Restructuring the code so that the ir_variables could be in parameters
then move them to the instruction stream was... enough to make a grown
Mick Jagger cry.

I'm not terribly enamored with this solution either, but I didn't see a
better way.

>> +
>> +      ir_function *const func =
>> +         shader->symbols->get_function("__intrinsic_atomic_add");
>> +      ir_instruction *const c = call(func, retval, parameters);
>> +
>> +      assert(c != NULL);
>> +      assert(parameters.is_empty());
>> +
>> +      body.emit(c);
>> +   } else {
>> +      body.emit(call(shader->symbols->get_function(intrinsic), retval,
>> +                     sig->parameters));
>> +   }
>> +
>>     body.emit(ret(retval));
>>     return sig;
>>  }
>> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> index 197b3af..3320f2a 100644
>> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> @@ -3208,13 +3208,6 @@ glsl_to_tgsi_visitor::visit_atomic_counter_intrinsic(ir_call *ir)
>>           val = ((ir_instruction *)param)->as_rvalue();
>>           val->accept(this);
>>           data2 = this->result;
>> -      } else if (!strcmp("__intrinsic_atomic_sub", callee)) {
>> -         opcode = TGSI_OPCODE_ATOMUADD;
>> -         st_src_reg res = get_temp(glsl_type::uvec4_type);
>> -         st_dst_reg dstres = st_dst_reg(res);
>> -         dstres.writemask = dst.writemask;
>> -         emit_asm(ir, TGSI_OPCODE_INEG, dstres, data);
>> -         data = res;
>>        } else {
>>           assert(!"Unexpected intrinsic");
>>           return;
>> @@ -3625,7 +3618,6 @@ glsl_to_tgsi_visitor::visit(ir_call *ir)
>>         !strcmp("__intrinsic_atomic_increment", callee) ||
>>         !strcmp("__intrinsic_atomic_predecrement", callee) ||
>>         !strcmp("__intrinsic_atomic_add", callee) ||
>> -       !strcmp("__intrinsic_atomic_sub", callee) ||
>>         !strcmp("__intrinsic_atomic_min", callee) ||
>>         !strcmp("__intrinsic_atomic_max", callee) ||
>>         !strcmp("__intrinsic_atomic_and", callee) ||
>> --
>> 2.5.5
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>