Re: [Mesa-dev] [PATCH 4/5] nir: Translate image load, store and atomic intrinsics from GLSL IR.

2015-05-08 Thread Connor Abbott 
On Fri, May 8, 2015 at 10:30 AM, Francisco Jerez  wrote:
> Connor Abbott  writes:
>
>> On Tue, May 5, 2015 at 4:29 PM, Francisco Jerez  
>> wrote:
>>> ---
>>>  src/glsl/nir/glsl_to_nir.cpp | 125 
>>> +++
>>>  1 file changed, 114 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp
>>> index f6b8331..a01ab3b 100644
>>> --- a/src/glsl/nir/glsl_to_nir.cpp
>>> +++ b/src/glsl/nir/glsl_to_nir.cpp
>>> @@ -614,27 +614,130 @@ nir_visitor::visit(ir_call *ir)
>>>   op = nir_intrinsic_atomic_counter_inc_var;
>>>} else if (strcmp(ir->callee_name(), 
>>> "__intrinsic_atomic_predecrement") == 0) {
>>>   op = nir_intrinsic_atomic_counter_dec_var;
>>> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_load") == 0) 
>>> {
>>> + op = nir_intrinsic_image_load;
>>> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_store") == 
>>> 0) {
>>> + op = nir_intrinsic_image_store;
>>> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_atomic_add") 
>>> == 0) {
>>> + op = nir_intrinsic_image_atomic_add;
>>> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_atomic_min") 
>>> == 0) {
>>> + op = nir_intrinsic_image_atomic_min;
>>> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_atomic_max") 
>>> == 0) {
>>> + op = nir_intrinsic_image_atomic_max;
>>> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_atomic_and") 
>>> == 0) {
>>> + op = nir_intrinsic_image_atomic_and;
>>> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_atomic_or") 
>>> == 0) {
>>> + op = nir_intrinsic_image_atomic_or;
>>> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_atomic_xor") 
>>> == 0) {
>>> + op = nir_intrinsic_image_atomic_xor;
>>> +  } else if (strcmp(ir->callee_name(), 
>>> "__intrinsic_image_atomic_exchange") == 0) {
>>> + op = nir_intrinsic_image_atomic_exchange;
>>> +  } else if (strcmp(ir->callee_name(), 
>>> "__intrinsic_image_atomic_comp_swap") == 0) {
>>> + op = nir_intrinsic_image_atomic_comp_swap;
>>>} else {
>>>   unreachable("not reached");
>>>}
>>>
>>>nir_intrinsic_instr *instr = nir_intrinsic_instr_create(shader, op);
>>> -  ir_dereference *param =
>>> - (ir_dereference *) ir->actual_parameters.get_head();
>>> -  instr->variables[0] = evaluate_deref(&instr->instr, param);
>>> -  nir_ssa_dest_init(&instr->instr, &instr->dest, 1, NULL);
>>> +
>>> +  switch (op) {
>>> +  case nir_intrinsic_atomic_counter_read_var:
>>> +  case nir_intrinsic_atomic_counter_inc_var:
>>> +  case nir_intrinsic_atomic_counter_dec_var: {
>>> + ir_dereference *param =
>>> +(ir_dereference *) ir->actual_parameters.get_head();
>>> + instr->variables[0] = evaluate_deref(&instr->instr, param);
>>> + nir_ssa_dest_init(&instr->instr, &instr->dest, 1, NULL);
>>> + break;
>>> +  }
>>> +  case nir_intrinsic_image_load:
>>> +  case nir_intrinsic_image_store:
>>> +  case nir_intrinsic_image_atomic_add:
>>> +  case nir_intrinsic_image_atomic_min:
>>> +  case nir_intrinsic_image_atomic_max:
>>> +  case nir_intrinsic_image_atomic_and:
>>> +  case nir_intrinsic_image_atomic_or:
>>> +  case nir_intrinsic_image_atomic_xor:
>>> +  case nir_intrinsic_image_atomic_exchange:
>>> +  case nir_intrinsic_image_atomic_comp_swap: {
>>> + nir_load_const_instr *instr_zero = 
>>> nir_load_const_instr_create(shader, 1);
>>> + instr_zero->value.u[0] = 0;
>>> + nir_instr_insert_after_cf_list(this->cf_node_list, 
>>> &instr_zero->instr);
>>> +
>>> + /* Set the image variable dereference. */
>>> + exec_node *param = ir->actual_parameters.get_head();
>>> + ir_dereference *image = (ir_dereference *)param;
>>> + const glsl_type *type =
>>> +image->variable_referenced()->type->without_array();
>>> +
>>> + instr->variables[0] = evaluate_deref(&instr->instr, image);
>>> + param = param->get_next();
>>> +
>>> + /* Set the address argument, extending the coordinate vector to 
>>> four
>>> +  * components.
>>> +  */
>>> + const nir_src src_addr = evaluate_rvalue((ir_dereference *)param);
>>> + nir_alu_instr *instr_addr = nir_alu_instr_create(shader, 
>>> nir_op_vec4);
>>> + nir_ssa_dest_init(&instr_addr->instr, &instr_addr->dest.dest, 4, 
>>> NULL);
>>> +
>>> + for (int i = 0; i < 4; i++) {
>>> +if (i < type->coordinate_components()) {
>>> +   instr_addr->src[i].src = src_addr;
>>> +   instr_addr->src[i].swizzle[0] = i;
>>> +} else {
>>> +   instr_addr->src[i].src = nir_src_for_ssa(&instr_zero->def);
>>
>> I think it would better convey the intent to create an

Re: [Mesa-dev] [PATCH 4/5] nir: Translate image load, store and atomic intrinsics from GLSL IR.

2015-05-08 Thread Francisco Jerez 
Connor Abbott  writes:

> On Tue, May 5, 2015 at 4:29 PM, Francisco Jerez  wrote:
>> ---
>>  src/glsl/nir/glsl_to_nir.cpp | 125 
>> +++
>>  1 file changed, 114 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp
>> index f6b8331..a01ab3b 100644
>> --- a/src/glsl/nir/glsl_to_nir.cpp
>> +++ b/src/glsl/nir/glsl_to_nir.cpp
>> @@ -614,27 +614,130 @@ nir_visitor::visit(ir_call *ir)
>>   op = nir_intrinsic_atomic_counter_inc_var;
>>} else if (strcmp(ir->callee_name(), 
>> "__intrinsic_atomic_predecrement") == 0) {
>>   op = nir_intrinsic_atomic_counter_dec_var;
>> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_load") == 0) {
>> + op = nir_intrinsic_image_load;
>> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_store") == 0) 
>> {
>> + op = nir_intrinsic_image_store;
>> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_atomic_add") 
>> == 0) {
>> + op = nir_intrinsic_image_atomic_add;
>> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_atomic_min") 
>> == 0) {
>> + op = nir_intrinsic_image_atomic_min;
>> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_atomic_max") 
>> == 0) {
>> + op = nir_intrinsic_image_atomic_max;
>> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_atomic_and") 
>> == 0) {
>> + op = nir_intrinsic_image_atomic_and;
>> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_atomic_or") 
>> == 0) {
>> + op = nir_intrinsic_image_atomic_or;
>> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_atomic_xor") 
>> == 0) {
>> + op = nir_intrinsic_image_atomic_xor;
>> +  } else if (strcmp(ir->callee_name(), 
>> "__intrinsic_image_atomic_exchange") == 0) {
>> + op = nir_intrinsic_image_atomic_exchange;
>> +  } else if (strcmp(ir->callee_name(), 
>> "__intrinsic_image_atomic_comp_swap") == 0) {
>> + op = nir_intrinsic_image_atomic_comp_swap;
>>} else {
>>   unreachable("not reached");
>>}
>>
>>nir_intrinsic_instr *instr = nir_intrinsic_instr_create(shader, op);
>> -  ir_dereference *param =
>> - (ir_dereference *) ir->actual_parameters.get_head();
>> -  instr->variables[0] = evaluate_deref(&instr->instr, param);
>> -  nir_ssa_dest_init(&instr->instr, &instr->dest, 1, NULL);
>> +
>> +  switch (op) {
>> +  case nir_intrinsic_atomic_counter_read_var:
>> +  case nir_intrinsic_atomic_counter_inc_var:
>> +  case nir_intrinsic_atomic_counter_dec_var: {
>> + ir_dereference *param =
>> +(ir_dereference *) ir->actual_parameters.get_head();
>> + instr->variables[0] = evaluate_deref(&instr->instr, param);
>> + nir_ssa_dest_init(&instr->instr, &instr->dest, 1, NULL);
>> + break;
>> +  }
>> +  case nir_intrinsic_image_load:
>> +  case nir_intrinsic_image_store:
>> +  case nir_intrinsic_image_atomic_add:
>> +  case nir_intrinsic_image_atomic_min:
>> +  case nir_intrinsic_image_atomic_max:
>> +  case nir_intrinsic_image_atomic_and:
>> +  case nir_intrinsic_image_atomic_or:
>> +  case nir_intrinsic_image_atomic_xor:
>> +  case nir_intrinsic_image_atomic_exchange:
>> +  case nir_intrinsic_image_atomic_comp_swap: {
>> + nir_load_const_instr *instr_zero = 
>> nir_load_const_instr_create(shader, 1);
>> + instr_zero->value.u[0] = 0;
>> + nir_instr_insert_after_cf_list(this->cf_node_list, 
>> &instr_zero->instr);
>> +
>> + /* Set the image variable dereference. */
>> + exec_node *param = ir->actual_parameters.get_head();
>> + ir_dereference *image = (ir_dereference *)param;
>> + const glsl_type *type =
>> +image->variable_referenced()->type->without_array();
>> +
>> + instr->variables[0] = evaluate_deref(&instr->instr, image);
>> + param = param->get_next();
>> +
>> + /* Set the address argument, extending the coordinate vector to 
>> four
>> +  * components.
>> +  */
>> + const nir_src src_addr = evaluate_rvalue((ir_dereference *)param);
>> + nir_alu_instr *instr_addr = nir_alu_instr_create(shader, 
>> nir_op_vec4);
>> + nir_ssa_dest_init(&instr_addr->instr, &instr_addr->dest.dest, 4, 
>> NULL);
>> +
>> + for (int i = 0; i < 4; i++) {
>> +if (i < type->coordinate_components()) {
>> +   instr_addr->src[i].src = src_addr;
>> +   instr_addr->src[i].swizzle[0] = i;
>> +} else {
>> +   instr_addr->src[i].src = nir_src_for_ssa(&instr_zero->def);
>
> I think it would better convey the intent to create an ssa_undef_instr
> and use that here instead of zero. Unless something else relies on the
> extra coordinates being zeroed?
>
Yeah, that would work too.  Zero makes some sense

Re: [Mesa-dev] [PATCH 4/5] nir: Translate image load, store and atomic intrinsics from GLSL IR.

2015-05-07 Thread Connor Abbott 
On Tue, May 5, 2015 at 4:29 PM, Francisco Jerez  wrote:
> ---
>  src/glsl/nir/glsl_to_nir.cpp | 125 
> +++
>  1 file changed, 114 insertions(+), 11 deletions(-)
>
> diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp
> index f6b8331..a01ab3b 100644
> --- a/src/glsl/nir/glsl_to_nir.cpp
> +++ b/src/glsl/nir/glsl_to_nir.cpp
> @@ -614,27 +614,130 @@ nir_visitor::visit(ir_call *ir)
>   op = nir_intrinsic_atomic_counter_inc_var;
>} else if (strcmp(ir->callee_name(), 
> "__intrinsic_atomic_predecrement") == 0) {
>   op = nir_intrinsic_atomic_counter_dec_var;
> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_load") == 0) {
> + op = nir_intrinsic_image_load;
> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_store") == 0) {
> + op = nir_intrinsic_image_store;
> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_atomic_add") 
> == 0) {
> + op = nir_intrinsic_image_atomic_add;
> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_atomic_min") 
> == 0) {
> + op = nir_intrinsic_image_atomic_min;
> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_atomic_max") 
> == 0) {
> + op = nir_intrinsic_image_atomic_max;
> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_atomic_and") 
> == 0) {
> + op = nir_intrinsic_image_atomic_and;
> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_atomic_or") == 
> 0) {
> + op = nir_intrinsic_image_atomic_or;
> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_atomic_xor") 
> == 0) {
> + op = nir_intrinsic_image_atomic_xor;
> +  } else if (strcmp(ir->callee_name(), 
> "__intrinsic_image_atomic_exchange") == 0) {
> + op = nir_intrinsic_image_atomic_exchange;
> +  } else if (strcmp(ir->callee_name(), 
> "__intrinsic_image_atomic_comp_swap") == 0) {
> + op = nir_intrinsic_image_atomic_comp_swap;
>} else {
>   unreachable("not reached");
>}
>
>nir_intrinsic_instr *instr = nir_intrinsic_instr_create(shader, op);
> -  ir_dereference *param =
> - (ir_dereference *) ir->actual_parameters.get_head();
> -  instr->variables[0] = evaluate_deref(&instr->instr, param);
> -  nir_ssa_dest_init(&instr->instr, &instr->dest, 1, NULL);
> +
> +  switch (op) {
> +  case nir_intrinsic_atomic_counter_read_var:
> +  case nir_intrinsic_atomic_counter_inc_var:
> +  case nir_intrinsic_atomic_counter_dec_var: {
> + ir_dereference *param =
> +(ir_dereference *) ir->actual_parameters.get_head();
> + instr->variables[0] = evaluate_deref(&instr->instr, param);
> + nir_ssa_dest_init(&instr->instr, &instr->dest, 1, NULL);
> + break;
> +  }
> +  case nir_intrinsic_image_load:
> +  case nir_intrinsic_image_store:
> +  case nir_intrinsic_image_atomic_add:
> +  case nir_intrinsic_image_atomic_min:
> +  case nir_intrinsic_image_atomic_max:
> +  case nir_intrinsic_image_atomic_and:
> +  case nir_intrinsic_image_atomic_or:
> +  case nir_intrinsic_image_atomic_xor:
> +  case nir_intrinsic_image_atomic_exchange:
> +  case nir_intrinsic_image_atomic_comp_swap: {
> + nir_load_const_instr *instr_zero = 
> nir_load_const_instr_create(shader, 1);
> + instr_zero->value.u[0] = 0;
> + nir_instr_insert_after_cf_list(this->cf_node_list, 
> &instr_zero->instr);
> +
> + /* Set the image variable dereference. */
> + exec_node *param = ir->actual_parameters.get_head();
> + ir_dereference *image = (ir_dereference *)param;
> + const glsl_type *type =
> +image->variable_referenced()->type->without_array();
> +
> + instr->variables[0] = evaluate_deref(&instr->instr, image);
> + param = param->get_next();
> +
> + /* Set the address argument, extending the coordinate vector to four
> +  * components.
> +  */
> + const nir_src src_addr = evaluate_rvalue((ir_dereference *)param);
> + nir_alu_instr *instr_addr = nir_alu_instr_create(shader, 
> nir_op_vec4);
> + nir_ssa_dest_init(&instr_addr->instr, &instr_addr->dest.dest, 4, 
> NULL);
> +
> + for (int i = 0; i < 4; i++) {
> +if (i < type->coordinate_components()) {
> +   instr_addr->src[i].src = src_addr;
> +   instr_addr->src[i].swizzle[0] = i;
> +} else {
> +   instr_addr->src[i].src = nir_src_for_ssa(&instr_zero->def);

I think it would better convey the intent to create an ssa_undef_instr
and use that here instead of zero. Unless something else relies on the
extra coordinates being zeroed?

> +}
> + }
> +
> + nir_instr_insert_after_cf_list(cf_node_list, &instr_addr->instr);
> + instr->src[0] = nir_src_for_ssa(&instr_addr->dest.dest.ssa);
> +

[Mesa-dev] [PATCH 4/5] nir: Translate image load, store and atomic intrinsics from GLSL IR.

2015-05-05 Thread Francisco Jerez 
---
 src/glsl/nir/glsl_to_nir.cpp | 125 +++
 1 file changed, 114 insertions(+), 11 deletions(-)

diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp
index f6b8331..a01ab3b 100644
--- a/src/glsl/nir/glsl_to_nir.cpp
+++ b/src/glsl/nir/glsl_to_nir.cpp
@@ -614,27 +614,130 @@ nir_visitor::visit(ir_call *ir)
  op = nir_intrinsic_atomic_counter_inc_var;
   } else if (strcmp(ir->callee_name(), "__intrinsic_atomic_predecrement") 
== 0) {
  op = nir_intrinsic_atomic_counter_dec_var;
+  } else if (strcmp(ir->callee_name(), "__intrinsic_image_load") == 0) {
+ op = nir_intrinsic_image_load;
+  } else if (strcmp(ir->callee_name(), "__intrinsic_image_store") == 0) {
+ op = nir_intrinsic_image_store;
+  } else if (strcmp(ir->callee_name(), "__intrinsic_image_atomic_add") == 
0) {
+ op = nir_intrinsic_image_atomic_add;
+  } else if (strcmp(ir->callee_name(), "__intrinsic_image_atomic_min") == 
0) {
+ op = nir_intrinsic_image_atomic_min;
+  } else if (strcmp(ir->callee_name(), "__intrinsic_image_atomic_max") == 
0) {
+ op = nir_intrinsic_image_atomic_max;
+  } else if (strcmp(ir->callee_name(), "__intrinsic_image_atomic_and") == 
0) {
+ op = nir_intrinsic_image_atomic_and;
+  } else if (strcmp(ir->callee_name(), "__intrinsic_image_atomic_or") == 
0) {
+ op = nir_intrinsic_image_atomic_or;
+  } else if (strcmp(ir->callee_name(), "__intrinsic_image_atomic_xor") == 
0) {
+ op = nir_intrinsic_image_atomic_xor;
+  } else if (strcmp(ir->callee_name(), 
"__intrinsic_image_atomic_exchange") == 0) {
+ op = nir_intrinsic_image_atomic_exchange;
+  } else if (strcmp(ir->callee_name(), 
"__intrinsic_image_atomic_comp_swap") == 0) {
+ op = nir_intrinsic_image_atomic_comp_swap;
   } else {
  unreachable("not reached");
   }
 
   nir_intrinsic_instr *instr = nir_intrinsic_instr_create(shader, op);
-  ir_dereference *param =
- (ir_dereference *) ir->actual_parameters.get_head();
-  instr->variables[0] = evaluate_deref(&instr->instr, param);
-  nir_ssa_dest_init(&instr->instr, &instr->dest, 1, NULL);
+
+  switch (op) {
+  case nir_intrinsic_atomic_counter_read_var:
+  case nir_intrinsic_atomic_counter_inc_var:
+  case nir_intrinsic_atomic_counter_dec_var: {
+ ir_dereference *param =
+(ir_dereference *) ir->actual_parameters.get_head();
+ instr->variables[0] = evaluate_deref(&instr->instr, param);
+ nir_ssa_dest_init(&instr->instr, &instr->dest, 1, NULL);
+ break;
+  }
+  case nir_intrinsic_image_load:
+  case nir_intrinsic_image_store:
+  case nir_intrinsic_image_atomic_add:
+  case nir_intrinsic_image_atomic_min:
+  case nir_intrinsic_image_atomic_max:
+  case nir_intrinsic_image_atomic_and:
+  case nir_intrinsic_image_atomic_or:
+  case nir_intrinsic_image_atomic_xor:
+  case nir_intrinsic_image_atomic_exchange:
+  case nir_intrinsic_image_atomic_comp_swap: {
+ nir_load_const_instr *instr_zero = 
nir_load_const_instr_create(shader, 1);
+ instr_zero->value.u[0] = 0;
+ nir_instr_insert_after_cf_list(this->cf_node_list, 
&instr_zero->instr);
+
+ /* Set the image variable dereference. */
+ exec_node *param = ir->actual_parameters.get_head();
+ ir_dereference *image = (ir_dereference *)param;
+ const glsl_type *type =
+image->variable_referenced()->type->without_array();
+
+ instr->variables[0] = evaluate_deref(&instr->instr, image);
+ param = param->get_next();
+
+ /* Set the address argument, extending the coordinate vector to four
+  * components.
+  */
+ const nir_src src_addr = evaluate_rvalue((ir_dereference *)param);
+ nir_alu_instr *instr_addr = nir_alu_instr_create(shader, nir_op_vec4);
+ nir_ssa_dest_init(&instr_addr->instr, &instr_addr->dest.dest, 4, 
NULL);
+
+ for (int i = 0; i < 4; i++) {
+if (i < type->coordinate_components()) {
+   instr_addr->src[i].src = src_addr;
+   instr_addr->src[i].swizzle[0] = i;
+} else {
+   instr_addr->src[i].src = nir_src_for_ssa(&instr_zero->def);
+}
+ }
+
+ nir_instr_insert_after_cf_list(cf_node_list, &instr_addr->instr);
+ instr->src[0] = nir_src_for_ssa(&instr_addr->dest.dest.ssa);
+ param = param->get_next();
+
+ /* Set the sample argument, which should be zero for single-sample
+  * images.
+  */
+ if (type->sampler_dimensionality == GLSL_SAMPLER_DIM_MS) {
+instr->src[1] = evaluate_rvalue((ir_dereference *)param);
+param = param->get_next();
+ } else {
+instr->src[1] = nir_src_for_ssa(&instr_zero->def);
+ }
+
+ /* Set the intr