Re: [Mesa-dev] [PATCH 03/22] glsl: Add a ubo_load expression type for fetches from UBOs.

2012-08-06 Thread Eric Anholt
Kenneth Graunke kenn...@whitecape.org writes:
 On 07/31/2012 03:01 PM, Eric Anholt wrote:
 diff --git a/src/glsl/ir.h b/src/glsl/ir.h
 index f019837..2807ba6 100644
 --- a/src/glsl/ir.h
 +++ b/src/glsl/ir.h
 @@ -1018,9 +1018,18 @@ enum ir_expression_operation {
 ir_binop_pow,
  
 /**
 +* Load a value the size of a given GLSL type from a uniform block.
 +*
 +* operand0 is the uniform block index in the linked shader.
 +* operand1 is a constant or variable byte offset within the

 Constant or variable...you don't say :)

 I'd just go with byte offset within the uniform block.

Err, yeah.  operand 0 should have been clarified to be ir_constant,
while the qualifiers on operand 1 weren't helping.


pgpRBju3VKZB4.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 03/22] glsl: Add a ubo_load expression type for fetches from UBOs.

2012-08-01 Thread Kenneth Graunke
On 07/31/2012 03:01 PM, Eric Anholt wrote:
 Drivers will probably want to be able to take UBO references in a
 shader like:
 
 uniform ubo1 {
 float a;
 float b;
 float c;
 float d;
 }
 
 void main() {
  gl_FragColor = vec4(a, b, c, d);
 }
 
 and generate a single aligned vec4 load out of the UBO.  For intel,
 this involves recognizing the shared offset of the aligned loads and
 CSEing them out.  Obviously that involves breaking things down to
 loads from an offset from a particular UBO first.  Thus, the driver
 doesn't want to see
 
   variable_ref(ir_variable(a)),
 
 and even more so does it not want to see
 
   array_ref(record_ref(variable_ref(ir_variable(a)),
   field1), variable_ref(ir_variable(i))).
 
 where a.field1[i] is a row_major matrix.
 
 Instead, we're going to make a lowering pass to break UBO references
 down to expressions that are obvious to codegen, and amenable to
 merging through CSE.
 
 Reviewed-by: Ian Romanick ian.d.roman...@intel.com
 ---
  src/glsl/ir.cpp  |1 +
  src/glsl/ir.h|   11 ++-
  src/glsl/ir_validate.cpp |7 +++
  src/mesa/drivers/dri/i965/brw_fs_channel_expressions.cpp |5 +
  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |4 
  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp   |4 
  src/mesa/program/ir_to_mesa.cpp  |4 
  src/mesa/state_tracker/st_glsl_to_tgsi.cpp   |4 
  8 files changed, 39 insertions(+), 1 deletion(-)
 
 diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
 index b0e38d8..f59cdd2 100644
 --- a/src/glsl/ir.cpp
 +++ b/src/glsl/ir.cpp
 @@ -480,6 +480,7 @@ static const char *const operator_strs[] = {
 min,
 max,
 pow,
 +   ubo_load,
 vector,
  };
  
 diff --git a/src/glsl/ir.h b/src/glsl/ir.h
 index f019837..2807ba6 100644
 --- a/src/glsl/ir.h
 +++ b/src/glsl/ir.h
 @@ -1018,9 +1018,18 @@ enum ir_expression_operation {
 ir_binop_pow,
  
 /**
 +* Load a value the size of a given GLSL type from a uniform block.
 +*
 +* operand0 is the uniform block index in the linked shader.
 +* operand1 is a constant or variable byte offset within the

Constant or variable...you don't say :)

I'd just go with byte offset within the uniform block.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 03/22] glsl: Add a ubo_load expression type for fetches from UBOs.

2012-07-31 Thread Eric Anholt
Drivers will probably want to be able to take UBO references in a
shader like:

uniform ubo1 {
float a;
float b;
float c;
float d;
}

void main() {
 gl_FragColor = vec4(a, b, c, d);
}

and generate a single aligned vec4 load out of the UBO.  For intel,
this involves recognizing the shared offset of the aligned loads and
CSEing them out.  Obviously that involves breaking things down to
loads from an offset from a particular UBO first.  Thus, the driver
doesn't want to see

variable_ref(ir_variable(a)),

and even more so does it not want to see

array_ref(record_ref(variable_ref(ir_variable(a)),
  field1), variable_ref(ir_variable(i))).

where a.field1[i] is a row_major matrix.

Instead, we're going to make a lowering pass to break UBO references
down to expressions that are obvious to codegen, and amenable to
merging through CSE.

Reviewed-by: Ian Romanick ian.d.roman...@intel.com
---
 src/glsl/ir.cpp  |1 +
 src/glsl/ir.h|   11 ++-
 src/glsl/ir_validate.cpp |7 +++
 src/mesa/drivers/dri/i965/brw_fs_channel_expressions.cpp |5 +
 src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |4 
 src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp   |4 
 src/mesa/program/ir_to_mesa.cpp  |4 
 src/mesa/state_tracker/st_glsl_to_tgsi.cpp   |4 
 8 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
index b0e38d8..f59cdd2 100644
--- a/src/glsl/ir.cpp
+++ b/src/glsl/ir.cpp
@@ -480,6 +480,7 @@ static const char *const operator_strs[] = {
min,
max,
pow,
+   ubo_load,
vector,
 };
 
diff --git a/src/glsl/ir.h b/src/glsl/ir.h
index f019837..2807ba6 100644
--- a/src/glsl/ir.h
+++ b/src/glsl/ir.h
@@ -1018,9 +1018,18 @@ enum ir_expression_operation {
ir_binop_pow,
 
/**
+* Load a value the size of a given GLSL type from a uniform block.
+*
+* operand0 is the uniform block index in the linked shader.
+* operand1 is a constant or variable byte offset within the
+* uniform block.
+*/
+   ir_binop_ubo_load,
+
+   /**
 * A sentinel marking the last of the binary operations.
 */
-   ir_last_binop = ir_binop_pow,
+   ir_last_binop = ir_binop_ubo_load,
 
ir_quadop_vector,
 
diff --git a/src/glsl/ir_validate.cpp b/src/glsl/ir_validate.cpp
index 191d398..af0b576 100644
--- a/src/glsl/ir_validate.cpp
+++ b/src/glsl/ir_validate.cpp
@@ -423,6 +423,13 @@ ir_validate::visit_leave(ir_expression *ir)
   assert(ir-operands[0]-type == ir-operands[1]-type);
   break;
 
+   case ir_binop_ubo_load:
+  assert(ir-operands[0]-as_constant());
+  assert(ir-operands[0]-type == glsl_type::uint_type);
+
+  assert(ir-operands[1]-type == glsl_type::uint_type);
+  break;
+
case ir_quadop_vector:
   /* The vector operator collects some number of scalars and generates a
* vector from them.
diff --git a/src/mesa/drivers/dri/i965/brw_fs_channel_expressions.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_channel_expressions.cpp
index 983d92e..58521ee 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_channel_expressions.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_channel_expressions.cpp
@@ -337,6 +337,11 @@ ir_channel_expressions_visitor::visit_leave(ir_assignment 
*ir)
case ir_unop_noise:
   assert(!noise should have been broken down to function call);
   break;
+
+   case ir_binop_ubo_load:
+  assert(!not yet supported);
+  break;
+
case ir_quadop_vector:
   assert(!should have been lowered);
   break;
diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index fefe2c7..34f00ca 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -571,6 +571,10 @@ fs_visitor::visit(ir_expression *ir)
   else
 inst = emit(BRW_OPCODE_SHR, this-result, op[0], op[1]);
   break;
+
+   case ir_binop_ubo_load:
+  assert(!not yet supported);
+  break;
}
 }
 
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
index c77dc91..d6a786f 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
@@ -1314,6 +1314,10 @@ vec4_visitor::visit(ir_expression *ir)
 inst = emit(BRW_OPCODE_SHR, result_dst, op[0], op[1]);
   break;
 
+   case ir_binop_ubo_load:
+  assert(!not yet supported);
+  break;
+
case ir_quadop_vector:
   assert(!not reached: should be handled by lower_quadop_vector);
   break;
diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
index 255a8a7..70c4cc8 100644
---