[Mesa-dev] [PATCH 1/2] glsl: add lowering passes for carry/borrow

[Ilia Mirkin]

On Tue, Apr 29, 2014 at 12:37 AM, Matt Turner <mattst88 at gmail.com> wrote:
> On Mon, Apr 28, 2014 at 8:17 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> ---
>>
>> It didn't seem useful to add 2 separate lowering flags. I can't really imagine
>> wanting to lower one but not the other.
>
> I'd prefer to just have two flags.
>
>> Also, this is my first foray into the GLSL compiler, so please look out for
>> noob mistakes. This did pass piglit tests though.
>>
>>  src/glsl/ir_optimization.h      |  1 +
>>  src/glsl/lower_instructions.cpp | 55 +++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 56 insertions(+)
>>
>> diff --git a/src/glsl/ir_optimization.h b/src/glsl/ir_optimization.h
>> index 40bb613..5f0267d 100644
>> --- a/src/glsl/ir_optimization.h
>> +++ b/src/glsl/ir_optimization.h
>> @@ -38,6 +38,7 @@
>>  #define INT_DIV_TO_MUL_RCP 0x40
>>  #define BITFIELD_INSERT_TO_BFM_BFI 0x80
>>  #define LDEXP_TO_ARITH     0x100
>> +#define CARRY_TO_ARITH     0x200
>>
>>  /**
>>   * \see class lower_packing_builtins_visitor
>> diff --git a/src/glsl/lower_instructions.cpp b/src/glsl/lower_instructions.cpp
>> index 49316d0..ecbda96 100644
>> --- a/src/glsl/lower_instructions.cpp
>> +++ b/src/glsl/lower_instructions.cpp
>> @@ -39,6 +39,7 @@
>>   * - MOD_TO_FRACT
>>   * - LDEXP_TO_ARITH
>>   * - BITFIELD_INSERT_TO_BFM_BFI
>> + * - CARRY_TO_ARITH
>>   *
>>   * SUB_TO_ADD_NEG:
>>   * ---------------
>> @@ -91,6 +92,10 @@
>>   * Breaks ir_quadop_bitfield_insert into ir_binop_bfm (bitfield mask) and
>>   * ir_triop_bfi (bitfield insert).
>>   *
>> + * CARRY_TO_ARITH:
>> + * ---------------
>> + * Converts ir_carry and ir_borrow into (x + y) > x and (x < y)
>> + *
>>   * Many GPUs implement the bitfieldInsert() built-in from ARB_gpu_shader_5
>>   * with a pair of instructions.
>>   *
>> @@ -127,6 +132,8 @@ private:
>>     void log_to_log2(ir_expression *);
>>     void bitfield_insert_to_bfm_bfi(ir_expression *);
>>     void ldexp_to_arith(ir_expression *);
>> +   void carry_to_arith(ir_expression *);
>> +   void borrow_to_arith(ir_expression *);
>>  };
>>
>>  } /* anonymous namespace */
>> @@ -436,6 +443,44 @@ lower_instructions_visitor::ldexp_to_arith(ir_expression *ir)
>>     this->progress = true;
>>  }
>>
>> +void
>> +lower_instructions_visitor::carry_to_arith(ir_expression *ir)
>> +{
>> +   /* Translates
>> +    *   ir_binop_carry x y
>> +    * into
>> +    *   sum = ir_binop_add x y
>> +    *   bcarry = ir_binop_less sum x
>> +    *   carry = ir_unop_b2i bcarry
>> +    */
>> +
>> +   ir_rvalue *x_clone = ir->operands[0]->clone(ir, NULL);
>> +   ir->operation = ir_unop_i2u;
>> +   ir->operands[0] = b2i(less(
>> +         add(ir->operands[0], ir->operands[1]),
>> +         x_clone));
>
> Weird indentation.

Seems reasonable to me. But this all fits on one line now, so I'll
just do that :) [It didn't before, when I had ir->operands[0] in there
twice.]

>
> With another flag, better indentation, and fixed descriptions
>
> Reviewed-by: Matt Turner <mattst88 at gmail.com>

Thanks, will resend just in case.