Re: [Mesa-dev] [PATCH] TransformFeedback: Assign transform feedback varying slots in linker.
On 10/23/2011 01:19 PM, Marek Olšák wrote:
On Sat, Oct 22, 2011 at 2:14 AM, Dan McCabezen3d.li...@gmail.com wrote:
Modify the linker to assign additional slots for varying
variables used by transform feedback. This is done after other
varyings are already assigned slots.
Since this is done after previous varying slot assignments,
the code needs to know how many varyings are already assigned
slots. A new function max_varying() is introduced to examine a
previously processed shader to find the largest varying already
assigned a slot. All new varyings will be assigned slots after
this one. If varyings are found, -1 is returned.
---
src/glsl/linker.cpp | 84 +++
1 files changed, 84 insertions(+), 0 deletions(-)
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index a7c38a3..81f2658 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -207,6 +207,25 @@ invalidate_variable_locations(gl_shader *sh, enum
ir_variable_mode mode,
}
}
+
+int
+max_varying(gl_shader *sh, enum ir_variable_mode mode)
+{
+ int max_varying = -1;
+
+ foreach_list(node, sh-ir) {
+ ir_variable *const var = ((ir_instruction *) node)-as_variable();
+
+ if ((var == NULL) || (var-mode != (unsigned) mode))
+continue;
+
+ if (var-location max_varying)
+max_varying = var-location;
+ }
+
+ return max_varying;
+}
+
/**
* Determine the number of attribute slots required for a particular type
@@ -1597,6 +1616,69 @@ assign_varying_locations(struct gl_context *ctx,
void
+assign_transform_feedback_varying_locations(struct gl_context *ctx,
+ struct gl_shader_program *prog)
+{
+ struct gl_shader *vs = prog-_LinkedShaders[MESA_SHADER_VERTEX];
+
+ if (vs == NULL)
+ return;
+
+ char **names = prog-TransformFeedback.VaryingNames;
+ int num_names = prog-TransformFeedback.NumVarying;
+
+ if (num_names= 0)
+ return;
+
+ int num_varying = max_varying(vs, ir_var_out) + 1;
+ unsigned output_index =
+ (num_varying VERT_RESULT_VAR0)
+? num_varying
+: VERT_RESULT_VAR0;
+
+ foreach_list(node, vs-ir) {
+ ir_variable *const output_var = ((ir_instruction *) node)-as_variable();
+
+ if (output_var == NULL
+ || output_var-mode != ir_var_out
+ || output_var-location != -1)
+continue;
+
+ /* Find a transform feedback varying variable that has
+ * the same name as the shader variable.
+ */
+ int varying_index = -1;
+ for (int num_name = 0; num_name num_names; num_name++) {
+ char *name = *names++;
Hi Dan,
Sorry for the very late reply and thank you very much for the work. I
have just a few remarks.
1) *names++ doesn't seem correct and causes a segfault. You probably
wanted *(names++). I fixed this in my code by using names[num_name],
which is more readable.
Agreed on both points. Thanks.
2) I am not sure whether your code will work with varying arrays. With
varying vec4 array[2];, array[0] and array[1] are possible names
for transform feedback varyings.
Possibly not; this hasn't been tested extensively. But I'll check.
3) If a transform feedback varying is undeclared, linker_error should
be called and the linker should back off.
Agreed.
Other than that, transform feedback varying locations seem to be
assigned correctly.
Best regards,
Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] TransformFeedback: Assign transform feedback varying slots in linker.
Modify the linker to assign additional slots for varying
variables used by transform feedback. This is done after other
varyings are already assigned slots.
Since this is done after previous varying slot assignments,
the code needs to know how many varyings are already assigned
slots. A new function max_varying() is introduced to examine a
previously processed shader to find the largest varying already
assigned a slot. All new varyings will be assigned slots after
this one. If varyings are found, -1 is returned.
---
src/glsl/linker.cpp | 84 +++
1 files changed, 84 insertions(+), 0 deletions(-)
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index a7c38a3..81f2658 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -207,6 +207,25 @@ invalidate_variable_locations(gl_shader *sh, enum
ir_variable_mode mode,
}
}
+
+int
+max_varying(gl_shader *sh, enum ir_variable_mode mode)
+{
+ int max_varying = -1;
+
+ foreach_list(node, sh-ir) {
+ ir_variable *const var = ((ir_instruction *) node)-as_variable();
+
+ if ((var == NULL) || (var-mode != (unsigned) mode))
+continue;
+
+ if (var-location max_varying)
+max_varying = var-location;
+ }
+
+ return max_varying;
+}
+
/**
* Determine the number of attribute slots required for a particular type
@@ -1597,6 +1616,69 @@ assign_varying_locations(struct gl_context *ctx,
void
+assign_transform_feedback_varying_locations(struct gl_context *ctx,
+ struct gl_shader_program *prog)
+{
+ struct gl_shader *vs = prog-_LinkedShaders[MESA_SHADER_VERTEX];
+
+ if (vs == NULL)
+ return;
+
+ char **names = prog-TransformFeedback.VaryingNames;
+ int num_names = prog-TransformFeedback.NumVarying;
+
+ if (num_names = 0)
+ return;
+
+ int num_varying = max_varying(vs, ir_var_out) + 1;
+ unsigned output_index =
+ (num_varying VERT_RESULT_VAR0)
+? num_varying
+: VERT_RESULT_VAR0;
+
+ foreach_list(node, vs-ir) {
+ ir_variable *const output_var = ((ir_instruction *) node)-as_variable();
+
+ if (output_var == NULL
+ || output_var-mode != ir_var_out
+ || output_var-location != -1)
+continue;
+
+ /* Find a transform feedback varying variable that has
+ * the same name as the shader variable.
+ */
+ int varying_index = -1;
+ for (int num_name = 0; num_name num_names; num_name++) {
+ char *name = *names++;
+ if (strcmp(output_var-name, name) == 0) {
+varying_index = num_name;
+break;
+ }
+ }
+
+ if (varying_index == -1)
+continue;
+
+ output_var-location = output_index;
+
+ /* FINISHME: Support for varying records in GLSL 1.50. */
+ assert(!output_var-type-is_record());
+
+ if (output_var-type-is_array()) {
+const unsigned slots = output_var-type-length
+ * output_var-type-fields.array-matrix_columns;
+
+output_index += slots;
+ } else {
+const unsigned slots = output_var-type-matrix_columns;
+
+output_index += slots;
+ }
+ }
+}
+
+
+void
link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
{
void *mem_ctx = ralloc_context(NULL); // temporary linker context
@@ -1782,6 +1864,8 @@ link_shaders(struct gl_context *ctx, struct
gl_shader_program *prog)
prev = i;
}
+ assign_transform_feedback_varying_locations(ctx, prog);
+
if (prog-_LinkedShaders[MESA_SHADER_VERTEX] != NULL) {
demote_shader_inputs_and_outputs(prog-_LinkedShaders[MESA_SHADER_VERTEX],
ir_var_out);
--
1.7.4.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/5] mesa: Make the gl_constant_value's bool occupy the same space as float/int.
What are the implications for other architectures that support doubles?
On 08/19/2011 05:56 PM, Eric Anholt wrote:
At least for Intel, all our uniform components are of uint32_t size, either
float or signed or unsigned int. For uploading uniform data in the driver,
it's much easier to upload a full dword per uniform element instead of trying
to pick out the bool byte and then fill in the top 3 bytes of pad with 0.
---
src/mesa/program/prog_parameter.h |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/mesa/program/prog_parameter.h
b/src/mesa/program/prog_parameter.h
index 1a5ed34..4c2773a 100644
--- a/src/mesa/program/prog_parameter.h
+++ b/src/mesa/program/prog_parameter.h
@@ -53,7 +53,7 @@
typedef union gl_constant_value
{
GLfloat f;
- GLboolean b;
+ GLint b;
GLint i;
GLuint u;
} gl_constant_value;
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/5] mesa: Fix glGetUniformfv of native integer uniforms.
On 08/20/2011 12:16 AM, Kenneth Graunke wrote:
On 08/19/2011 05:56 PM, Eric Anholt wrote:
We have to actually convert the values on the way out. Fixes piglit
ARB_shader_objects/getuniform.
---
src/mesa/main/uniforms.c | 32
1 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/src/mesa/main/uniforms.c b/src/mesa/main/uniforms.c
index cda840f..fbaa810 100644
--- a/src/mesa/main/uniforms.c
+++ b/src/mesa/main/uniforms.c
@@ -55,13 +55,11 @@ static GLenum
base_uniform_type(GLenum type)
{
switch (type) {
-#if 0 /* not needed, for now */
case GL_BOOL:
case GL_BOOL_VEC2:
case GL_BOOL_VEC3:
case GL_BOOL_VEC4:
return GL_BOOL;
-#endif
case GL_FLOAT:
case GL_FLOAT_VEC2:
case GL_FLOAT_VEC3:
@@ -408,8 +406,12 @@ get_uniform(struct gl_context *ctx, GLuint program, GLint
location,
else {
const struct gl_program_parameter *p =
prog-Parameters-Parameters[paramPos];
+ gl_constant_value (*values)[4];
GLint rows, cols, i, j, k;
GLsizei numBytes;
+ GLenum storage_type;
+
+ values = prog-Parameters-ParameterValues + paramPos + offset;
get_uniform_rows_cols(p,rows,cols);
@@ -421,15 +423,37 @@ get_uniform(struct gl_context *ctx, GLuint program, GLint
location,
return;
}
+ if (ctx-Const.NativeIntegers) {
+storage_type = base_uniform_type(p-DataType);
+ } else {
+storage_type = GL_FLOAT;
+ }
+
switch (returnType) {
case GL_FLOAT:
{
GLfloat *params = (GLfloat *) paramsOut;
k = 0;
for (i = 0; i rows; i++) {
- const int base = paramPos + offset + i;
for (j = 0; j cols; j++ ) {
- params[k++] = prog-Parameters-ParameterValues[base][j].f;
+ switch (storage_type) {
+ case GL_FLOAT:
+params[k++] = values[i][j].f;
+break;
+ case GL_INT:
+params[k++] = values[i][j].i;
+break;
+ case GL_UNSIGNED_INT:
+params[k++] = values[i][j].u;
+break;
+ case GL_BOOL:
+params[k++] = values[i][j].b;
+break;
+ default:
+_mesa_problem(ctx, Invalid type in glGetUniform());
+params[k++] = 0.0;
+break;
+ }
}
}
}
Argh. The OpenGL specification is so unclear on what happens when you
call GetUniform{f, i, ui}v on a value of a different type. Your code
here assumes that it's legal to call GetUniformfv on an int/bool/uint
uniform, and that the uniform's value should be implicitly converted to
floating point. That seems plausible.
But what about calling GetUniformiv or GetUniformuiv on a float uniform?
Is it supposed to convert the float value to an int/uint? By what
rounding rules...truncate? We're certainly not doing that now. If
that's the proper behavior, we need your switch statement in the GL_INT
and GL_UNSIGNED_INT returnType cases as well.
We also do need it for the GL_DOUBLE returnType case. (Admittedly
that's a GL 4 function we don't implement, but we may as well get it
right now...)
Ditto.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/5] mesa: Make the gl_constant_value's bool occupy the same space as float/int.
On 08/20/2011 01:30 PM, Bryan Cain wrote:
On 08/20/2011 03:05 PM, Dan McCabe wrote:
What are the implications for other architectures that support doubles?
I don't see what you mean. gl_constant_value doesn't support doubles yet.
Yet - that is the operative word.
You can buy GPUs that support doubles today. Therefore, double support
should be on our radar (it appears to be on Eric's radar, based on his
commit comment). And we should understand what the implications are for
our code. Clearly, I don't understand the implications; if I did, I
wouldn't have asked. But perhaps Eric might.
There are a couple of possible answers.
I don't know - OK, but at least let's ask the question and start
thinking about it's answer.
No impact - I like that answer.
It affects this, and this, and this - While not ideal, at least we
then know what to do in the future and prepare ourselves for that future.
cheers, danm
Bryan
On 08/19/2011 05:56 PM, Eric Anholt wrote:
At least for Intel, all our uniform components are of uint32_t size,
either
float or signed or unsigned int. For uploading uniform data in the
driver,
it's much easier to upload a full dword per uniform element instead
of trying
to pick out the bool byte and then fill in the top 3 bytes of pad
with 0.
---
src/mesa/program/prog_parameter.h |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/mesa/program/prog_parameter.h
b/src/mesa/program/prog_parameter.h
index 1a5ed34..4c2773a 100644
--- a/src/mesa/program/prog_parameter.h
+++ b/src/mesa/program/prog_parameter.h
@@ -53,7 +53,7 @@
typedef union gl_constant_value
{
GLfloat f;
- GLboolean b;
+ GLint b;
GLint i;
GLuint u;
} gl_constant_value;
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel: Emit assertion failure ASAP when DRI2 separate stencil handshake fails
On 08/19/2011 10:44 AM, Chad Versace wrote:
On 08/19/2011 10:35 AM, Eric Anholt wrote:
On Thu, 18 Aug 2011 14:02:46 -0700, Chad Versacec...@chad-versace.us wrote:
When intel_verify_dri2_has_hiz() discovers that DRI2 (that is, the DDX
driver) cannot provide a separate stencil buffer, but
intel_context::hw_must_use_separate_stencil is set, then emit an
informative assertion failure as soon as possible.
Currently, we do emit an assertion failure, but the its location is
sufficiently unrelated to the DRI2 HiZ handshake as to be uninformative.
In experimenting with HiZ, Anholt encountered this assertion failure and
was unable to understand its cause.
CC: Eric Anholte...@anholt.net
Signed-off-by: Chad Versacec...@chad-versace.us
---
src/mesa/drivers/dri/intel/intel_context.c |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/src/mesa/drivers/dri/intel/intel_context.c
b/src/mesa/drivers/dri/intel/intel_context.c
index fe8be08..b9d2579 100644
--- a/src/mesa/drivers/dri/intel/intel_context.c
+++ b/src/mesa/drivers/dri/intel/intel_context.c
@@ -1454,6 +1454,10 @@ intel_verify_dri2_has_hiz(struct intel_context *intel,
* a combined depth/stencil buffer. Discard the hiz buffer too.
*/
intel-intelScreen-dri2_has_hiz = INTEL_DRI2_HAS_HIZ_FALSE;
+if (intel-must_use_separate_stencil) {
+ assert(!intel_context requires separate stencil, but the
+ DRIscreen does not support it);
+}
If it's something we think people can actually hit (and in this case we
do), just use _mesa_problem so people see it even if they aren't
building with assertions.
I would also like to throw an assertion immediately after _mesa_problem(), like
below.
Since an assertion will eventually fail anyway, we might as well die ASAP.
_mesa_probelm(ctx, intel_context requires separate stencil, but the
DRIscreen does no support it. You may need to upgrade
the Intel X driver to 2.16.0);
the Intel X driver to 2.16.0 or newer);
assert(0);
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/4] i965/vs: Add simple dead code elimination.
On 08/18/2011 01:23 PM, Kenneth Graunke wrote:
On 08/18/2011 12:02 PM, Matt Turner wrote:
On Thu, Aug 18, 2011 at 2:38 PM, Eric Anholte...@anholt.net wrote:
+ bool progress = true;
+ while (progress) {
+ progress = false;
+ progress = dead_code_eliminate() || progress;
|| progress is always false, though maybe it's just written like that
to match the style of the code.
I'd have just written this as
bool progress;
do {
progress = dead_code_eliminate();
} while (progress)
Matt
The idea is that we'll add more passes later, so eventually, progress
won't just be false. Using the do-while loop is probably clearer though.
If that's the case (that we'll add more passes later), this should have
been noted in the commit msg. It would have prevented people from
scratching their heads about the logic.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] Implementing varying packing
On 08/16/2011 07:06 AM, vlj wrote: From: Vincent Lejeunev...@ovi.com This optimisation pass will look for and pack together float, vec2, vec3 varyings in fragment shaders and transform the vertex shader accordingly. It might improve performance depending on the hardware. Do you have any quatitative data on performance improvements you have actually seen? What sort of perf improvements should we expect? cheers, danm ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Fix type error when lowering integer divisions
On 08/15/2011 10:45 AM, Paul Berry wrote:
On 15 August 2011 10:11, Dan McCabezen3d.li...@gmail.com wrote:
You might also want to consider implementing
quotient = int((float(x) + 0.5 * float(y)) * reciprocal(float(y)));
This rounds the result to the nearest integer rather then flooring the
result and is arguably faster (assuming that common subexpressions for
float(y) are hoisted).
I see where you're coming from, and if we were designing a new
programming language from scratch, I might consider the idea. But I
don't feel like we can break from tradition that strongly. In every
programming language I'm aware of, dividing two integers to produce an
integer result either rounds toward zero (e.g. C99) or toward negative
infinity (e.g. Python 3.0, if you use the floor division operator).
Nobody rounds to nearest. And I believe I've seen code that relies on
this rounding behavior, for example in computing loop bounds:
// access an array in groups of three, ignoring any leftover bit at the end
for (int i = 0; i array_length/3; ++i) {
...access array elements 3*i, 3*i+1, and 3*i+2...
}
If we redefined integer division to do round to nearest, code like
the above would break.
Incidentally, my experiments so far with the nVidia Linux driver
indicate that it implements round toward zero behavior.
It's really too bad that the GLSL spec doesn't narrow down corner
cases like these, or make reference to standards that do. It seems
like their intention has been to make the language as C-like as
possible--how hard would it have been for them to say Integer
operations are defined as in C99?
Yeah, I agree with your points.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/5] glsl: Add switch statement support to the GLSL compiler.
On 08/03/2011 12:39 PM, Paul Berry wrote:
On 1 August 2011 17:29, Dan McCabezen3d.li...@gmail.com wrote:
This patch set adds support for switch statements to the GLSL compiler. We
modify the grammar for the compiler with productions for switch statements
and case labels, while adding supporting supporting productions not already
present. New AST classes are defined to support those productions. However,
with our approach no new IR is needed, allowing us to leverage all existing
optimizations and code generation.
Regarding the grammar, we note that the grammar as summarized in the appendix
of the GLSL specs leaves a bit to be desired. For example, it appears that case
labels can be used anywhere a statement is valid. However, we note that the
description of the switch statement in Section 6.2 in the body of the spec is
much more specific, and we follow that text to guide our creation of new
productions.
Specifically, we add productions for:
switch body,
case label list,
case statement, and
case statement list.
The switch body and the case statement allow us to limit where case labels may
be used.
In turn, we create new AST classes for each of these productions.
For code generation, we generate previously existing IR. Switch statements
can be thought of a series of if/then/else statements. Case labels are
compared with the value of a test expression and the case statements are
executed if the comparison is true.
There are a couple of aspects of switch statements that complicate this
simplistic view. The primary one is that cases can fall through sequentially
to subsequent cases, unless a break statement is encountered, in which case
the switch statement exits completely.
But break handling is further complicated by the fact that a break statement
can impact the exit of a loop. Thus, we need to coordinate break processing
between switch statements and loop statements.
The code generated by a switch statement maintains three temporary state
variables:
int test_value;
bool is_fallthru;
bool is_break;
test_value is initialized to the value of the test expression at the head of
the switch statement. This is the value that case labels are compared against.
is_fallthru is used to sequentially fall through to subsequent cases and is
initialized to false. When a case label matches the test expression, this
state variable is set to true. It will also be forced to false if a break
statement has been encountered. This forcing to false on break MUST be
after every case test. In practice, we defer that forcing to immediately after
the last case comparison prior to executing a case statement, but that is
an optimization.
is_break is used to indicate that a break statement has been executed and is
initialized to false. When a break statement is encountered, it is set to true.
This state variable is then used to conditionally force is_fallthru to to false
to prevent subsequent case statements from executing.
Code generation for break statements depends on whether the break statement is
inside a switch statement or inside a loop statement. If it inside a loop
statement is inside a break statement, the same code as before gets generated.
But if a switch statement is inside a loop statement, code is emitted to set
the is_break state to true.
Just as ASTs for loop statements are managed in a stack-like manner to handle
nesting, we also add a bool to capture the innermost switch or loop condition.
Note that we still need to maintain a loop AST stack to properly handle
for-loop code generation on a continue statement. Technically, we don't (yet)
need a switch AST stack, but we are using one for orthogonality with loop
statements and in anticipation of potential future use. Note that a simple
boolean stack would have sufficed.
We will illustrate a switch statement with its analogous conditional code that
a switch statement corresponds to by considering an example.
Consider the following switch statement:
switch (42) {
case 0:
case 1:
gl_FragColor = vec4(1.0, 2.0, 3.0, 4.0);
case 2:
case 3:
gl_FragColor = vec4(4.0, 3.0, 2.0, 1.0);
break;
case 4:
default:
gl_FragColor = vec4(0.0, 0.0, 0.0, 0.0);
}
Note that case 0 and case 1 fall through to cases 2 and 3 if they occur.
Note that case 4 and the default case must be reached explicitly, since cases
2 and 3 break at the end of their case.
Finally, note that case 4 and the default case don't break but simply fall
through to the end of the switch.
For this code, the equivalent code can be expressed as:
int test_val = 42; // capture value of test expression
bool is_fallthru = false; // prevent initial fall throughs
bool is_break = false; // capture the execution of a break stmt
is_fallthru |= (test_val == 0); // enable fallthru on case 0
is_fallthru |= (test_val == 1); // enable
[Mesa-dev] [PATCH 0/5] glsl: Add switch statement support to the GLSL compiler.
This patch set adds support for switch statements to the GLSL compiler. We
modify the grammar for the compiler with productions for switch statements
and case labels, while adding supporting supporting productions not already
present. New AST classes are defined to support those productions. However,
with our approach no new IR is needed, allowing us to leverage all existing
optimizations and code generation.
Regarding the grammar, we note that the grammar as summarized in the appendix
of the GLSL specs leaves a bit to be desired. For example, it appears that case
labels can be used anywhere a statement is valid. However, we note that the
description of the switch statement in Section 6.2 in the body of the spec is
much more specific, and we follow that text to guide our creation of new
productions.
Specifically, we add productions for:
switch body,
case label list,
case statement, and
case statement list.
The switch body and the case statement allow us to limit where case labels may
be used.
In turn, we create new AST classes for each of these productions.
For code generation, we generate previously existing IR. Switch statements
can be thought of a series of if/then/else statements. Case labels are
compared with the value of a test expression and the case statements are
executed if the comparison is true.
There are a couple of aspects of switch statements that complicate this
simplistic view. The primary one is that cases can fall through sequentially
to subsequent cases, unless a break statement is encountered, in which case
the switch statement exits completely.
But break handling is further complicated by the fact that a break statement
can impact the exit of a loop. Thus, we need to coordinate break processing
between switch statements and loop statements.
The code generated by a switch statement maintains three temporary state
variables:
int test_value;
bool is_fallthru;
bool is_break;
test_value is initialized to the value of the test expression at the head of
the switch statement. This is the value that case labels are compared against.
is_fallthru is used to sequentially fall through to subsequent cases and is
initialized to false. When a case label matches the test expression, this
state variable is set to true. It will also be forced to false if a break
statement has been encountered. This forcing to false on break MUST be
after every case test. In practice, we defer that forcing to immediately after
the last case comparison prior to executing a case statement, but that is
an optimization.
is_break is used to indicate that a break statement has been executed and is
initialized to false. When a break statement is encountered, it is set to true.
This state variable is then used to conditionally force is_fallthru to to false
to prevent subsequent case statements from executing.
Code generation for break statements depends on whether the break statement is
inside a switch statement or inside a loop statement. If it inside a loop
statement is inside a break statement, the same code as before gets generated.
But if a switch statement is inside a loop statement, code is emitted to set
the is_break state to true.
Just as ASTs for loop statements are managed in a stack-like manner to handle
nesting, we also add a bool to capture the innermost switch or loop condition.
Note that we still need to maintain a loop AST stack to properly handle
for-loop code generation on a continue statement. Technically, we don't (yet)
need a switch AST stack, but we are using one for orthogonality with loop
statements and in anticipation of potential future use. Note that a simple
boolean stack would have sufficed.
We will illustrate a switch statement with its analogous conditional code that
a switch statement corresponds to by considering an example.
Consider the following switch statement:
switch (42) {
case 0:
case 1:
gl_FragColor = vec4(1.0, 2.0, 3.0, 4.0);
case 2:
case 3:
gl_FragColor = vec4(4.0, 3.0, 2.0, 1.0);
break;
case 4:
default:
gl_FragColor = vec4(0.0, 0.0, 0.0, 0.0);
}
Note that case 0 and case 1 fall through to cases 2 and 3 if they occur.
Note that case 4 and the default case must be reached explicitly, since cases
2 and 3 break at the end of their case.
Finally, note that case 4 and the default case don't break but simply fall
through to the end of the switch.
For this code, the equivalent code can be expressed as:
int test_val = 42; // capture value of test expression
bool is_fallthru = false; // prevent initial fall throughs
bool is_break = false; // capture the execution of a break stmt
is_fallthru |= (test_val == 0); // enable fallthru on case 0
is_fallthru |= (test_val == 1); // enable fallthru on case 1
is_fallthru = !is_break; // inhibit fallthru on previous break
[Mesa-dev] [PATCH 1/5] glsl: Create AST data structures for switch statement and case label
Data structures for switch statement and case label are created that parallel
the structure of other AST data.
---
src/glsl/ast.h | 24
1 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/src/glsl/ast.h b/src/glsl/ast.h
index 878f48b..2ee0b11 100644
--- a/src/glsl/ast.h
+++ b/src/glsl/ast.h
@@ -628,13 +628,19 @@ public:
class ast_case_label : public ast_node {
public:
+ ast_case_label(ast_expression *test_value);
+ virtual void print(void) const;
+
+ virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
/**
-* An expression of NULL means 'default'.
+* An test value of NULL means 'default'.
*/
- ast_expression *expression;
+ ast_expression *test_value;
};
+
class ast_selection_statement : public ast_node {
public:
ast_selection_statement(ast_expression *condition,
@@ -653,8 +659,18 @@ public:
class ast_switch_statement : public ast_node {
public:
- ast_expression *expression;
- exec_list statements;
+ ast_switch_statement(ast_expression *test_expression,
+ ast_node *body);
+ virtual void print(void) const;
+
+ virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
+
+ ast_expression *test_expression;
+ ast_node *body;
+
+protected:
+ void test_to_hir(exec_list *, struct _mesa_glsl_parse_state *);
};
class ast_iteration_statement : public ast_node {
--
1.7.4.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/5] glsl: Add productions to GLSL grammar for switch statement
The grammar is modified to support switch statements. Rather than follow the
grammar in the appendix, which allows case labels to be placed ANYWHERE
as a regular statement, we follow the development of the grammar as
described in the body of the GLSL spec.
In this variation, the switch statement has a body which consists of a list
of case statements. A case statement is preceded by a list of case labels and
ends with a list of statements.
---
src/glsl/glsl_parser.yy | 64 --
1 files changed, 61 insertions(+), 3 deletions(-)
diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
index 2c0498e..b3727ce 100644
--- a/src/glsl/glsl_parser.yy
+++ b/src/glsl/glsl_parser.yy
@@ -206,6 +206,12 @@
%type declaration struct_declarator_list
%type node selection_statement
%type selection_rest_statement selection_rest_statement
+%type node switch_statement
+%type node switch_body
+%type node case_label
+%type node case_label_list
+%type node case_statement
+%type node case_statement_list
%type node iteration_statement
%type node condition
%type node conditionopt
@@ -1519,8 +1525,7 @@ simple_statement:
declaration_statement
| expression_statement
| selection_statement
- | switch_statement { $$ = NULL; }
- | case_label{ $$ = NULL; }
+ | switch_statement
| iteration_statement
| jump_statement
;
@@ -1642,15 +1647,68 @@ condition:
}
;
+/*
+ * siwtch_statement grammar is based on the syntax described in the body
+ * of the GLSL spec, not in it's appendix!!!
+ */
switch_statement:
- SWITCH '(' expression ')' compound_statement
+ SWITCH '(' expression ')' switch_body
+ {
+ $$ = NULL;
+ }
+ ;
+
+switch_body:
+ '{' '}'
+ {
+ $$ = NULL;
+ }
+ | '{' case_statement_list '}'
+ {
+ $$ = NULL;
+ }
;
case_label:
CASE expression ':'
+ {
+ $$ = NULL;
+ }
| DEFAULT ':'
+ {
+ $$ = NULL;
+ }
;
+case_label_list:
+ case_label
+ {
+ $$ = NULL;
+ }
+ | case_label_list case_label
+ {
+ $$ = NULL;
+ }
+ ;
+
+case_statement:
+ case_label_list statement_list
+ {
+ $$ = NULL;
+ }
+ ;
+
+case_statement_list:
+ case_statement
+ {
+ $$ = NULL;
+ }
+ | case_statement_list case_statement
+ {
+ $$ = NULL;
+ }
+ ;
+
iteration_statement:
WHILE '(' condition ')' statement_no_new_scope
{
--
1.7.4.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/5] glsl: Create AST structs corresponding to new productions in grammar
Previously we added productions for:
switch_body
case_label_list
case_statement
case_statement_list
Now add AST structs corresponding to those productions.
---
src/glsl/ast.h | 59
1 files changed, 59 insertions(+), 0 deletions(-)
diff --git a/src/glsl/ast.h b/src/glsl/ast.h
index 2ee0b11..6568f17 100644
--- a/src/glsl/ast.h
+++ b/src/glsl/ast.h
@@ -641,6 +641,65 @@ public:
};
+class ast_case_label_list : public ast_node {
+public:
+ ast_case_label_list(void);
+ virtual void print(void) const;
+
+ virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
+
+ /**
+* A list of case labels.
+*/
+ exec_list labels;
+};
+
+
+class ast_case_statement : public ast_node {
+public:
+ ast_case_statement(ast_case_label_list *labels);
+ virtual void print(void) const;
+
+ virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
+
+ ast_case_label_list *labels;
+
+ /**
+* A list of statements.
+*/
+ exec_list stmts;
+};
+
+
+class ast_case_statement_list : public ast_node {
+public:
+ ast_case_statement_list(void);
+ virtual void print(void) const;
+
+ virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
+
+ /**
+* A list of cases.
+*/
+ exec_list cases;
+};
+
+
+class ast_switch_body : public ast_node {
+public:
+ ast_switch_body(ast_case_statement_list *stmts);
+ virtual void print(void) const;
+
+ virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
+
+ ast_case_statement_list *stmts;
+};
+
+
class ast_selection_statement : public ast_node {
public:
ast_selection_statement(ast_expression *condition,
--
1.7.4.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] intel: Fix span functions for stencil buffer
On 07/09/2011 08:56 AM, Chad Versace wrote:
Up until this point, we incorrectly believed that the stencil buffer is
Y-tiled. In fact, it is W tiled. From PRM Vol 1 Part 2 Section 4.5.2.1
W-Major Tile Format:
W-Major Tile Format is used for separate stencil.
Since the stencil buffer is allocated with I915_TILING_Y, the span
functions must decode W tiling through a Y tiled fence.
On gen5 with intel_screen.hw_must_use_separate_stencil enabled,
Fixes-Piglit-test: stencil-drawpixels
Fixes-Piglit-test: stencil-scissor-clear
Fixes-Piglit-test: readpixels-24_8
Note: This is a candidate for the 7.11 branch
Signed-off-by: Chad Versacec...@chad-versace.us
---
src/mesa/drivers/dri/intel/intel_span.c | 52 +--
1 files changed, 35 insertions(+), 17 deletions(-)
diff --git a/src/mesa/drivers/dri/intel/intel_span.c
b/src/mesa/drivers/dri/intel/intel_span.c
index 153803f..f39c008 100644
--- a/src/mesa/drivers/dri/intel/intel_span.c
+++ b/src/mesa/drivers/dri/intel/intel_span.c
@@ -141,28 +141,46 @@ intel_set_span_functions(struct intel_context *intel,
/**
* \brief Get pointer offset into stencil buffer.
*
- * The stencil buffer interleaves two rows into one. Yay for crazy hardware.
- * The table below demonstrates how the pointer arithmetic behaves for a buffer
- * with positive stride (s=stride).
- *
- * x| y | byte offset
- * --
- * 0| 0 | 0
- * 0| 1 | 1
- * 1| 0 | 2
- * 1| 1 | 3
- * ... | ... | ...
- * 0| 2 | s
- * 0| 3 | s + 1
- * 1| 2 | s + 2
- * 1| 3 | s + 3
- *
+ * The stencil buffer is W-tiled, yet the drm buffer is allocated with
+ * I915_TILING_Y. So here we must decode the W tiling through a Y fence.
*
+ * From PRM Vol 1 Part 2 Section 4.5.2.1 W-Major Tile Format:
+ * W-Major Tile Format is used for separate stencil.
*/
static inline intptr_t
intel_offset_S8(int stride, GLint x, GLint y)
{
- return 2 * ((y / 2) * stride + x) + y % 2;
+ /* f: (x, y) - (fx, fy) */
+ int fx = x / 8;
+ int fy = y / 4;
+
+ /* e: (x, y) - (ex, 0) */
+ int ex = (x % 8) / 4;
+
+ /* d: (x, y) - (dx, dy) */
+ int dx = (x % 4) / 2;
+ int dy = (y % 4) / 2;
+
+ /* c: (x, y) - (cx, cy) */
+ int cx = x % 2;
+ int cy = y % 2;
+
+ int s = stride;
+ intptr_t o = 0;
+
+ if (s 0) {
+ /*f*/ o += 16 * fx + 4 * s * fy;
+ /*e*/ o += 2 * s * ex;
+ /*d*/ o += 4 * dx + 8 * dy;
+ /*c*/ o += cx + 2 * cy;
+ } else {
+ /*f*/ o += 16 * fx + 4 * s * fy;
+ /*e*/ o += 2 * s * (1 - ex);
+ /*d*/ o += 4 * dx + 8 * (1 - dy);
+ /*c*/ o += cx + 2 * (1 - cy);
+ }
+
+ return o;
}
#define WRITE_STENCIL(x, y, src) buf[intel_offset_S8(stride, x, y)] = src;
Can stride ever be negative? If so, why? If the app ever specified a
negative stride, it could have fixed at buffer creation time (by also
adjusting the buffer base address). No need to worry about that issue
thereafter. On the other hand, negative strides could be considered evil :).
Also, can x or y ever be negative? I'm assuming these address
manipulations are performed after clipping and/or scissoring, in which
case, they would always be non-negative (but that assertion should be
confirmed, preferably prior to compile time).
Using *, / and % for bit manipulations of pixel addresses should
be avoided. Shifts and masks are clearer for bit manipulation, IMO.
Regarding the offset computation, it might be useful to think of x,y
addresses of the tile and then of x,y addresses within the tile to make
the code more readable and perhaps simplify your computations.
For example, if stride is a power of 2, tile size is 2x2 pixels, and
your x and y address bits look like (upper case bits are tile bits and
lower case letters are intra-tile bits):
Xx
and
Yy
then the offset for that pixel and a power of two stride has a bit
pattern that looks like
yx
But the devil is in the details and this might not be valid for our
particular (crazy?) hardware. YMMV.
cheers, danm
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] intel: Fix span functions for stencil buffer
On 07/11/2011 04:03 PM, Chad Versace wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 07/11/2011 12:49 PM, Dan McCabe wrote:
On 07/09/2011 08:56 AM, Chad Versace wrote:
Up until this point, we incorrectly believed that the stencil buffer is
Y-tiled. In fact, it is W tiled. From PRM Vol 1 Part 2 Section 4.5.2.1
W-Major Tile Format:
W-Major Tile Format is used for separate stencil.
Since the stencil buffer is allocated with I915_TILING_Y, the span
functions must decode W tiling through a Y tiled fence.
On gen5 with intel_screen.hw_must_use_separate_stencil enabled,
Fixes-Piglit-test: stencil-drawpixels
Fixes-Piglit-test: stencil-scissor-clear
Fixes-Piglit-test: readpixels-24_8
Note: This is a candidate for the 7.11 branch
Signed-off-by: Chad Versacec...@chad-versace.us
---
src/mesa/drivers/dri/intel/intel_span.c | 52
+--
1 files changed, 35 insertions(+), 17 deletions(-)
diff --git a/src/mesa/drivers/dri/intel/intel_span.c
b/src/mesa/drivers/dri/intel/intel_span.c
index 153803f..f39c008 100644
--- a/src/mesa/drivers/dri/intel/intel_span.c
+++ b/src/mesa/drivers/dri/intel/intel_span.c
@@ -141,28 +141,46 @@ intel_set_span_functions(struct intel_context *intel,
/**
* \brief Get pointer offset into stencil buffer.
*
- * The stencil buffer interleaves two rows into one. Yay for crazy hardware.
- * The table below demonstrates how the pointer arithmetic behaves for a buffer
- * with positive stride (s=stride).
- *
- * x| y | byte offset
- * --
- * 0| 0 | 0
- * 0| 1 | 1
- * 1| 0 | 2
- * 1| 1 | 3
- * ... | ... | ...
- * 0| 2 | s
- * 0| 3 | s + 1
- * 1| 2 | s + 2
- * 1| 3 | s + 3
- *
+ * The stencil buffer is W-tiled, yet the drm buffer is allocated with
+ * I915_TILING_Y. So here we must decode the W tiling through a Y fence.
*
+ * From PRM Vol 1 Part 2 Section 4.5.2.1 W-Major Tile Format:
+ * W-Major Tile Format is used for separate stencil.
*/
static inline intptr_t
intel_offset_S8(int stride, GLint x, GLint y)
{
- return 2 * ((y / 2) * stride + x) + y % 2;
+ /* f: (x, y) - (fx, fy) */
+ int fx = x / 8;
+ int fy = y / 4;
+
+ /* e: (x, y) - (ex, 0) */
+ int ex = (x % 8) / 4;
+
+ /* d: (x, y) - (dx, dy) */
+ int dx = (x % 4) / 2;
+ int dy = (y % 4) / 2;
+
+ /* c: (x, y) - (cx, cy) */
+ int cx = x % 2;
+ int cy = y % 2;
+
+ int s = stride;
+ intptr_t o = 0;
+
+ if (s 0) {
+ /*f*/ o += 16 * fx + 4 * s * fy;
+ /*e*/ o += 2 * s * ex;
+ /*d*/ o += 4 * dx + 8 * dy;
+ /*c*/ o += cx + 2 * cy;
+ } else {
+ /*f*/ o += 16 * fx + 4 * s * fy;
+ /*e*/ o += 2 * s * (1 - ex);
+ /*d*/ o += 4 * dx + 8 * (1 - dy);
+ /*c*/ o += cx + 2 * (1 - cy);
+ }
+
+ return o;
}
#define WRITE_STENCIL(x, y, src) buf[intel_offset_S8(stride, x, y)] = src;
Can stride ever be negative? If so, why?
Yes. The stride is negative for window-system renderbuffers.
If the app ever specified a negative stride, it could have fixed at buffer
creation time (by also adjusting the buffer base address). No need to worry
about that issue thereafter.
No-can-do. As far as I know, X demands that its buffers have negative stride.
On the other hand, negative strides could be considered evil :).
Yes, they are :)
Also, can x or y ever be negative?
No.
[snip]
Using *, / and % for bit manipulations of pixel addresses should be
avoided. Shifts and masks are clearer for bit manipulation, IMO.
Regarding the offset computation, it might be useful to think of x,y addresses
of the tile and then of x,y addresses within the tile to make the code more
readable and perhaps simplify your computations.
Damn crazy hardware. Here we need to decode a W tile through a Y fence, so the x,y
addresses of the tile and then of x,y addresses within the tile are non-existent.
The fence mapping has carved up the W tile and made a mess of it. So, there are no tile
addresses to compute.
Here is the little meaning that my equations possess:
- (fx + ex, fy) is the address of a 4x4 block in which (x, y) resides.
- Decompose that 4x4 block into 2x2 blocks. (dx, dy) is
the address of that 2x2 block within the 4x4 block.
- (cx, cy) is the address of (x, y) within that 2x2 block.
For example, if stride is a power of 2, tile size is 2x2 pixels, and your x and
y address bits look like (upper case bits are tile bits and lower case letters
are intra-tile bits):
Xx
and
Yy
then the offset for that pixel and a power of two stride has a bit pattern that
looks like
yx
But the devil is in the details and this might not be valid for our particular
(crazy?) hardware. YMMV.
Can a similiar set of bit operations replicate intel_offset_S8()?
Yeah. I started working out the details, but stopped after concluding
[Mesa-dev] Building internal glsl_compiler with builtin_function.o instead of builtin-stubs.o?
Is there a good reason not to build the internal glsl_compiler (in mesa/src/glsl) and link it with builtin_function.o? It is currently being built with builtin_stubs.o. I can understand why builtin_compiler uses builtin_stubs.o instead of builtin_function.o (to avoid conflicts while building builtin_functions.cpp, which is why builtin_compiler exists in the first place). But we build BOTH builtin-compiler and glsl_compiler, and having the latter build with actual builtin functions instead of nothing makes it useful to debug the code generated by the builtin functions (as well as other tests that rely on the builtin functions). TIA cheers, danm ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/5] glsl: Create AST structs corresponding to new productions in grammar
On 06/28/2011 10:47 PM, Kenneth Graunke wrote: On 06/28/2011 02:48 PM, Dan McCabe wrote: Previously we added productions for: switch_body case_label_list case_statement case_statement_list Now add AST structs corresponding to those productions. Both 1/3 and 3/3 look good. You might actually want to squash them...they're so similar. I'd order them like this: [PATCH 1/4] glsl: Add productions to GLSL grammar for switch statements [PATCH 2/4] glsl: Create AST structures for switch statements [PATCH 3/4] glsl: Reference data structure ctors in grammar [PATCH 4/4] glsl: Generate IR for switch statements My reasoning for ordering them the way I did is that 1/5 was the least amount of code the **COULD** be added. 2/5 included the discovery that the naive approach to the grammar doesn't work and that you needed something more complex. 3/5 then adds the support for the realistic variant of the grammar. I tried to grow the complexity organically and order them with the intent that if you nothing about the code (as was the case for me when I started this work), you would gradually learn all that was needed to make it work in a logical progression. But your suggested re-ordering makes sense if you already know the code base. When 4/5 and 5/5 get reviewed, I will revisit the issue. It's not a big deal to me either way. cheers, danm ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 5/5] glsl: Generate IR for switch statements
Right back at ya', comments in-line, in response to your in-line comments.
Thanks for the good feedback and review.
cheers, danm
On 06/29/2011 12:53 PM, Kenneth Graunke wrote:
On 06/28/2011 02:48 PM, Dan McCabe wrote:
Up until now modifying the GLSL compiler has been pretty straightforward.
This is where things get interesting. But still pretty straightforward.
Dan,
This patch looks good! I found a few small issues...nothing major. I
think one more round and it should be ready to go. Comments inline.
Switch statements can be thought of a series of if/then/else statements.
Case labels are compared with the value of a test expression and the case
statements are executed if the comparison is true.
There are a couple of aspects of switch statements that complicate this simple
view of the world. The primary one is that cases can fall through sequentially
to subsequent case, unless a break statement is encountered, in which case,
the switch statement exits completely.
But break handling is further complicated by the fact that a break statement
can impact the exit of a loop. Thus, we need to coordinate break processing
between switch statements and loop statements.
The code generated by a switch statement maintains three temporary state
variables:
int test_value;
bool is_fallthru;
bool is_break;
test_value is initialized to the value of the test expression at the head of
the switch statement. This is the value that case labels are compared against.
is_fallthru is used to sequentially fall through to subsequent cases and is
initialized to false. When a case label matches the test expression, this
state variable is set to true. It will also be forced to false if a break
statement has been encountered. This forcing to false on break MUST be
after every case test. In practice, we defer that forcing to immediately after
the last case comparison prior to executing a case statement, but that is
an optimization.
is_break is used to indicate that a break statement has been executed and is
initialized to false. When a break statement is encountered, it is set to true.
This state variable is then used to conditionally force is_fallthru to to false
to prevent subsequent case statements from executing.
Code generation for break statements depends on whether the break statement is
inside a switch statement or inside a loop statement. If it inside a loop
statement is inside a break statement, the same code as before gets generated.
But if a switch statement is inside a loop statement, code is emitted to set
the is_break state to true.
Just as ASTs for loop statements are managed in a stack-like
manner to handle nesting, we also add a bool to capture the innermost switch
or loop condition. Note that we still need to maintain a loop AST stack to
properly handle for-loop code generation on a continue statement. Technically,
we don't (yet) need a switch AST stack, but I am using one for orthogonality
with loop statements, in anticipation of future use. Note that a simple
boolean stack would have sufficed.
We will illustrate a switch statement with its analogous conditional code that
a switch statement corresponds to by examining an example.
Consider the following switch statement:
switch (42) {
case 0:
case 1:
gl_FragColor = vec4(1.0, 2.0, 3.0, 4.0);
case 2:
case 3:
gl_FragColor = vec4(4.0, 3.0, 2.0, 1.0);
break;
case 4:
default:
gl_FragColor = vec4(0.0, 0.0, 0.0, 0.0);
}
Note that case 0 and case 1 fall through to cases 2 and 3 if they occur.
Note that case 4 and the default case must be reached explicitly, since cases
2 and 3 break at the end of their case.
Finally, note that case 4 and the default case don't break but simply fall
through to the end of the switch.
For this code, the equivalent code can be expressed as:
int test_val = 42; // capture value of test expression
bool is_fallthru = false; // prevent initial fall through
bool is_break = false; // capture the execution of a break stmt
is_fallthru |= (test_val == 0); // enable fallthru on case 0
is_fallthru |= (test_val == 1); // enable fallthru on case 1
is_fallthru= !is_break; // inhibit fallthru on previous break
if (is_fallthru) {
gl_FragColor = vec4(1.0, 2.0, 3.0, 4.0);
}
is_fallthru |= (test_val == 2); // enable fallthru on case 2
is_fallthru |= (test_val == 3); // enable fallthru on case 3
is_fallthru= !is_break; // inhibit fallthru on previous break
if (is_fallthru) {
gl_FragColor = vec4(4.0, 3.0, 2.0, 1.0);
is_break = true; // inhibit all subsequent fallthru for break
}
is_fallthru |= (test_val == 4); // enable fallthru on case 4
is_fallthru = true; // enable fallthru for default case
is_fallthru= !is_break; // inhibit
Re: [Mesa-dev] [PATCH 6/6] glsl: Generate IR for switch statements
On 06/28/2011 09:12 AM, Paul Berry wrote: On 27 June 2011 17:35, Keith Packardkei...@keithp.com wrote: I'd write the simplest possible code that works now and then consider optimizing it later. This has the advantage that you can get a bunch of tests working and then use those to validate a later, more complicated, implementation. I heartily support this plan. As my first CS prof used to say, get it right first, then make it fast. In general, I agree. Occam's Razor is always a useful device for choosing between two paths. However, as indicated to Keith, the request/suggestion by Kenneth looked very attractive, and indeed, once implemented, resulted in less smaller code. It was simpler to implement than what I had before and improves code quality as well. Tastes great, less filling. cheers, danm ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 0/5] glsl: Add switch statement support to GLSL compiler
This patch set adds support for switch statements to the GLSL compiler. We
modify the grammar for the compiler with productions for switch statements
and case labels, while adding supporting supporting productions not already
present. New AST classes are defined to support those productions. However,
with our approach no new IR is needed, allowing us to leverage all existing
optimizations and code generation.
Regarding the grammar, we note that the grammar as summarized in the appendix
of the GLSL specs leaves a bit to be desired. For example, it appears that case
labels can be used anywhere a statement is valid. However, we note that the
description of the switch statement in Section 6.2 in the body of the spec is
much more specific, and we follow that text to guide our creation of new
productions.
Specifically, we add productions for:
switch body,
case label list,
case statement, and
case statement list.
The switch body and the case statement allow us to limit where case labels may
be used.
In turn, we create new AST classes for each of these productions.
For code generation, we generate previously existing IR. Switch statements
can be thought of a series of if/then/else statements. Case labels are
compared with the value of a test expression and the case statements are
executed if the comparison is true.
There are a couple of aspects of switch statements that complicate this
simplistic view. The primary one is that cases can fall through sequentially
to subsequent cases, unless a break statement is encountered, in which case
the switch statement exits completely.
But break handling is further complicated by the fact that a break statement
can impact the exit of a loop. Thus, we need to coordinate break processing
between switch statements and loop statements.
The code generated by a switch statement maintains three temporary state
variables:
int test_value;
bool is_fallthru;
bool is_break;
test_value is initialized to the value of the test expression at the head of
the switch statement. This is the value that case labels are compared against.
is_fallthru is used to sequentially fall through to subsequent cases and is
initialized to false. When a case label matches the test expression, this
state variable is set to true. It will also be forced to false if a break
statement has been encountered. This forcing to false on break MUST be
after every case test. In practice, we defer that forcing to immediately after
the last case comparison prior to executing a case statement, but that is
an optimization.
is_break is used to indicate that a break statement has been executed and is
initialized to false. When a break statement is encountered, it is set to true.
This state variable is then used to conditionally force is_fallthru to to false
to prevent subsequent case statements from executing.
Code generation for break statements depends on whether the break statement is
inside a switch statement or inside a loop statement. If it inside a loop
statement is inside a break statement, the same code as before gets generated.
But if a switch statement is inside a loop statement, code is emitted to set
the is_break state to true.
Just as ASTs for loop statements are managed in a stack-like manner to handle
nesting, we also add a bool to capture the innermost switch or loop condition.
Note that we still need to maintain a loop AST stack to properly handle
for-loop code generation on a continue statement. Technically, we don't (yet)
need a switch AST stack, but we are using one for orthogonality with loop
statements and in anticipation of potential future use. Note that a simple
boolean stack would have sufficed.
We will illustrate a switch statement with its analogous conditional code that
a switch statement corresponds to by considering an example.
Consider the following switch statement:
switch (42) {
case 0:
case 1:
gl_FragColor = vec4(1.0, 2.0, 3.0, 4.0);
case 2:
case 3:
gl_FragColor = vec4(4.0, 3.0, 2.0, 1.0);
break;
case 4:
default:
gl_FragColor = vec4(0.0, 0.0, 0.0, 0.0);
}
Note that case 0 and case 1 fall through to cases 2 and 3 if they occur.
Note that case 4 and the default case must be reached explicitly, since cases
2 and 3 break at the end of their case.
Finally, note that case 4 and the default case don't break but simply fall
through to the end of the switch.
For this code, the equivalent code can be expressed as:
int test_val = 42; // capture value of test expression
bool is_fallthru = false; // prevent initial fall throughs
bool is_break = false; // capture the execution of a break stmt
is_fallthru |= (test_val == 0); // enable fallthru on case 0
is_fallthru |= (test_val == 1); // enable fallthru on case 1
is_fallthru = !is_break; // inhibit fallthru on previous break
[Mesa-dev] [PATCH 2/5] glsl: Add productions to GLSL grammar for switch statement
The grammar is modified to support switch statements. Rather than follow the
grammar in the appendix, which allows case labels to be placed ANYWHERE
as a regular statement, we follow the development of the grammar as
described in the body of the GLSL.
In this variation, the switch statement has a body which consists of a list
of case statements. A case statement is preceded by a list of case labels and
ends with a list of statements.
---
src/glsl/glsl_parser.yy | 64 --
1 files changed, 61 insertions(+), 3 deletions(-)
diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
index 2c0498e..b3727ce 100644
--- a/src/glsl/glsl_parser.yy
+++ b/src/glsl/glsl_parser.yy
@@ -206,6 +206,12 @@
%type declaration struct_declarator_list
%type node selection_statement
%type selection_rest_statement selection_rest_statement
+%type node switch_statement
+%type node switch_body
+%type node case_label
+%type node case_label_list
+%type node case_statement
+%type node case_statement_list
%type node iteration_statement
%type node condition
%type node conditionopt
@@ -1519,8 +1525,7 @@ simple_statement:
declaration_statement
| expression_statement
| selection_statement
- | switch_statement { $$ = NULL; }
- | case_label{ $$ = NULL; }
+ | switch_statement
| iteration_statement
| jump_statement
;
@@ -1642,15 +1647,68 @@ condition:
}
;
+/*
+ * siwtch_statement grammar is based on the syntax described in the body
+ * of the GLSL spec, not in it's appendix!!!
+ */
switch_statement:
- SWITCH '(' expression ')' compound_statement
+ SWITCH '(' expression ')' switch_body
+ {
+ $$ = NULL;
+ }
+ ;
+
+switch_body:
+ '{' '}'
+ {
+ $$ = NULL;
+ }
+ | '{' case_statement_list '}'
+ {
+ $$ = NULL;
+ }
;
case_label:
CASE expression ':'
+ {
+ $$ = NULL;
+ }
| DEFAULT ':'
+ {
+ $$ = NULL;
+ }
;
+case_label_list:
+ case_label
+ {
+ $$ = NULL;
+ }
+ | case_label_list case_label
+ {
+ $$ = NULL;
+ }
+ ;
+
+case_statement:
+ case_label_list statement_list
+ {
+ $$ = NULL;
+ }
+ ;
+
+case_statement_list:
+ case_statement
+ {
+ $$ = NULL;
+ }
+ | case_statement_list case_statement
+ {
+ $$ = NULL;
+ }
+ ;
+
iteration_statement:
WHILE '(' condition ')' statement_no_new_scope
{
--
1.7.4.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/5] glsl: Create AST structs corresponding to new productions in grammar
Previously we added productions for:
switch_body
case_label_list
case_statement
case_statement_list
Now add AST structs corresponding to those productions.
---
src/glsl/ast.h | 59
1 files changed, 59 insertions(+), 0 deletions(-)
diff --git a/src/glsl/ast.h b/src/glsl/ast.h
index 2ee0b11..6568f17 100644
--- a/src/glsl/ast.h
+++ b/src/glsl/ast.h
@@ -641,6 +641,65 @@ public:
};
+class ast_case_label_list : public ast_node {
+public:
+ ast_case_label_list(void);
+ virtual void print(void) const;
+
+ virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
+
+ /**
+* A list of case labels.
+*/
+ exec_list labels;
+};
+
+
+class ast_case_statement : public ast_node {
+public:
+ ast_case_statement(ast_case_label_list *labels);
+ virtual void print(void) const;
+
+ virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
+
+ ast_case_label_list *labels;
+
+ /**
+* A list of statements.
+*/
+ exec_list stmts;
+};
+
+
+class ast_case_statement_list : public ast_node {
+public:
+ ast_case_statement_list(void);
+ virtual void print(void) const;
+
+ virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
+
+ /**
+* A list of cases.
+*/
+ exec_list cases;
+};
+
+
+class ast_switch_body : public ast_node {
+public:
+ ast_switch_body(ast_case_statement_list *stmts);
+ virtual void print(void) const;
+
+ virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
+
+ ast_case_statement_list *stmts;
+};
+
+
class ast_selection_statement : public ast_node {
public:
ast_selection_statement(ast_expression *condition,
--
1.7.4.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/5] glsl: Reference data structure ctors in grammar
We now tie the grammar to the ctors of the ASTs they reference.
This requires that we actually have definitions of the ctors.
In addition, we also need to define print and hir methods for the AST
classes. The Print methods are pretty simple to flesh out. However, at this
stage of the development, we simply stub out the hir methods and flesh
them out later.
Also, since actual class instances get returned by the productions in the
grammar, we also need to designate the type of the productions that
reference those instances.
---
src/glsl/ast_to_hir.cpp | 54 +
src/glsl/glsl_parser.yy | 55 +++--
src/glsl/glsl_parser_extras.cpp | 100 +++
3 files changed, 193 insertions(+), 16 deletions(-)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 3b87f0d..d8ebd87 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -3274,6 +3274,60 @@ ast_selection_statement::hir(exec_list *instructions,
}
+ir_rvalue *
+ast_switch_statement::hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state)
+{
+ // FINISHME
+ return NULL;
+}
+
+
+ir_rvalue *
+ast_switch_body::hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state)
+{
+ // FINISHME
+ return NULL;
+}
+
+
+ir_rvalue *
+ast_case_statement::hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state)
+{
+ // FINISHME
+ return NULL;
+}
+
+
+ir_rvalue *
+ast_case_statement_list::hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state)
+{
+ // FINISHME
+ return NULL;
+}
+
+
+ir_rvalue *
+ast_case_label::hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state)
+{
+ // FINISHME
+ return NULL;
+}
+
+
+ir_rvalue *
+ast_case_label_list::hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state)
+{
+ // FINISHME
+ return NULL;
+}
+
+
void
ast_iteration_statement::condition_to_hir(ir_loop *stmt,
struct _mesa_glsl_parse_state *state)
diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
index b3727ce..fc3541c 100644
--- a/src/glsl/glsl_parser.yy
+++ b/src/glsl/glsl_parser.yy
@@ -67,6 +67,11 @@
ast_declarator_list *declarator_list;
ast_struct_specifier *struct_specifier;
ast_declaration *declaration;
+ ast_switch_body *switch_body;
+ ast_case_label *case_label;
+ ast_case_label_list *case_label_list;
+ ast_case_statement *case_statement;
+ ast_case_statement_list *case_statement_list;
struct {
ast_node *cond;
@@ -207,11 +212,11 @@
%type node selection_statement
%type selection_rest_statement selection_rest_statement
%type node switch_statement
-%type node switch_body
-%type node case_label
-%type node case_label_list
-%type node case_statement
-%type node case_statement_list
+%type switch_body switch_body
+%type case_label_list case_label_list
+%type case_label case_label
+%type case_statement case_statement
+%type case_statement_list case_statement_list
%type node iteration_statement
%type node condition
%type node conditionopt
@@ -1654,58 +1659,76 @@ condition:
switch_statement:
SWITCH '(' expression ')' switch_body
{
- $$ = NULL;
+ $$ = new(state) ast_switch_statement($3, $5);
}
;
switch_body:
'{' '}'
{
- $$ = NULL;
+ $$ = new(state) ast_switch_body(NULL);
+ $$-set_location(yylloc);
}
| '{' case_statement_list '}'
{
- $$ = NULL;
+ $$ = new(state) ast_switch_body($2);
+ $$-set_location(yylloc);
}
;
case_label:
CASE expression ':'
{
- $$ = NULL;
+ $$ = new(state) ast_case_label($2);
}
| DEFAULT ':'
{
- $$ = NULL;
+ $$ = new(state) ast_case_label(NULL);
}
;
case_label_list:
case_label
{
- $$ = NULL;
+ ast_case_label_list *labels = new(state) ast_case_label_list();
+
+ labels-labels.push_tail( $1-link);
+ $$ = labels;
}
| case_label_list case_label
{
- $$ = NULL;
+ $$ = $1;
+ $$-labels.push_tail( $2-link);
}
;
case_statement:
- case_label_list statement_list
+ case_label_list statement
{
- $$ = NULL;
+ ast_case_statement *stmts = new(state) ast_case_statement($1);
+
+ stmts-stmts.push_tail( $2-link);
+ $$ = stmts
+ }
+ | case_statement statement
+ {
+ $$ = $1;
+ $$-stmts.push_tail( $2-link);
}
;
case_statement_list:
case_statement
{
- $$ = NULL;
+ ast_case_statement_list *cases= new(state) ast_case_statement_list();
Re: [Mesa-dev] [PATCH 0/6] glsl: Add support for switch statements
Comments at the end (where they are supposed to be :). On 06/17/2011 05:54 PM, Dan McCabe wrote: There are three changes from the patch set I published on 6/15: 1) Removed IR pointers from AST classes and moved them to glsl_parser_state, 2) Manage the new IR in a stack-like manner to properly handle nesting, and 3) Squash 6/7 and 7/7 together, since both deal with IR generation #1 was in response to the concerns Kenneth expressed from his code review on 6/15 about embedding IR data into AST structures. #2 was need as a consequence of #1 to support nested switch statements correctly. #3 is a minor cosmetic change that merges the final implicit break IR insertion into the larger collection of IR changes. cheers, danm On 06/17/2011 05:43 PM, Dan McCabe wrote: This patch set adds support for switch statements to the GLSL compiler. We modify the grammar for the compiler with productions for switch statements and case labels, while adding supporting supporting productions not already present. New AST classes are defined to support those productions. However, with our apporach no new IR is needed, allowing us to leverage all existing optimizations and code generation. A foolish inconsistency is the hobgoblin of small minds. Anyway, I just sent out pass 3 of this patch set. It now consists of 5 (not 6) pieces. Parts 4/6 and 5/6 got mashed together into 4/5, but that's not a big deal since the former part 5/6 was simply fleshing out print support for the AST. This change set has mostly minor, cosmetic changes. UNTIL part 5/5. The major issue that idr identified concerning nested loops should now be resolved. Basically, the loop model for switch statements got tossed into the waste basket. The reason I used loops in the first place was to leverage code generation for break statements. Bad idea. I now generate different code for break statements that are under the influence of a switch statement. Now, the switch statement is translated into straightforward if/then/else IR. In addition to maintaining internal state for the cached test value and the fall through state, I now additionally maintain a break state, which is initialized to false and set to true whenever a break stmt is executed. Prior to executing any case statements (the statements following a case label), the fallthru state is reset if the break state is enabled, inhibiting entry of the case statement block (which is guarded by the fallthru state). A couple of other changes were replacing the if/then/elses used to maintain the fallthru and break states with conditional assignments. This actually simplified both the src code and the generated code. As always, feedback is requested. cheers, danm ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 6/6] glsl: Generate IR for switch statements
On 06/27/2011 05:15 PM, Kenneth Graunke wrote:
On 06/24/2011 05:11 PM, Dan McCabe wrote:
Looking at a translation of my canonical example:
switch (expr) {
case c0:
case c1:
stmt0;
case c2:
case c3:
stmt1;
break;
case c4:
default:
stmt2;
}
We can translated this into:
int test_val_tmp = expr;
bool is_fallthru_tmp = false;
bool is_break_tmp = false;
if (test_val_tmp == c0) {
is_fallthru_tmp = true;
}
if (test_val_tmp == c1) {
is_fallthru_tmp = true;
}
if (is_break_tmp) {
is_fallthru_tmp = false;
}
if (is_fallthru_tmp) {
stmt0;
}
if (test_val_tmp == c2) {
is_fallthru_tmp = true;
}
if (test_val_tmp == c3) {
is_fallthru_tmp = true;
}
if (is_break_tmp) {
is_fallthru_tmp = false;
}
if (is_fallthru_tmp) {
stmt1;
is_break_tmp = true; // break stmt
}
if (test_val_tmp == c4) {
is_fallthru_tmp = true;
}
is_fallthru_tmp = true; // default
if (is_break_tmp) {
is_fallthru_tmp = false;
}
if (is_fallthru_tmp) {
stmt2;
}
I must admit, I'm not particularly crazy about the amount of
if-statements being generated here. A couple of ideas...
1. Use conditional moves:
(assign (ir_expression all_equal (var_ref test_val_tmp) c0) ; condition
(var_ref is_fallthru_tmp) ; assignee
(constant bool (1))) ; true
Then again Ian's new optimization pass should do this for us, so maybe
leaving it as if-statements is fine.
Since we'd have two conditional moves (for c0 and c1) with the same
target and value, it'd be nice to combine them...but that sounds like
a pretty obscure optimization pass.
2. Use ORs:
is_fallthru_tmp = false;
is_fallthru_tmp = is_fallthru_tmp || test_val_tmp == c0;
is_fallthru_tmp = is_fallthru_tmp || test_val_tmp == c1;
This seems somewhat easier to optimize...I imagine tree-grafting,
constant/copy propagation, etc. might be able to reduce this to:
is_fallthru_tmp = test_val_tmp == c0 || test_val_tmp == c1;
Again, I'm not sure. Ian, do you have an opinion?
In the end, this may be a moot point---we eventually want to take
advantage of hardware jump instructions. When switching on a uniform
(I'm guessing this is common), all the channels would take the same
path, so a simple ADD to the IP would work. Even for non-uniform
control flow, I suspect we could use the Gen break instruction,
which will jump if all channels take the path. But that's for later.
--Kenneth
Since I am just about to start modifying the IR generation, I'm open to
suggestion.
#1 looks like a better choice. But I might change my mind when I
actually get to modifying the code.
I originally considered #2, but the IR that was generated by such code
didn't look any better (i.e., you would up with a bunch of IF code
anyway). That is why I went the route I did.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 6/6] glsl: Generate IR for switch statements
On 06/27/2011 04:40 PM, Kenneth Graunke wrote:
Here's another case that I'm not sure you're handling
correctly...conditional breaks:
switch (expr) {
case c0:
case c1:
stmt0;
case c2:
case c3:
stmt1;
break;
case c4:
stmt2;
if (foo)
break;
stmt3;// happens if !foo
case c5:
default:
stmt4;// happens if (expr != c4 || foo)
};
The easiest solution I can think of is to (in C++) track whether a
break -might- have been taken and emit a guard around further statements:
stmt2;
if (foo)
has_break_tmp = true; // hit the break statement
if (has_break_tmp) { // could have potentially hit break, must check
stmt3;
}
This is similar to the proposal that I floated Fri PM and that you
commented upon at 17:15 today.
Please correct me if I'm wrong.
cheers, danm
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 6/6] glsl: Generate IR for switch statements
On 06/27/2011 05:35 PM, Keith Packard wrote: On Mon, 27 Jun 2011 17:23:30 -0700, Dan McCabezen3d.li...@gmail.com wrote: Since I am just about to start modifying the IR generation, I'm open to suggestion. I'd write the simplest possible code that works now and then consider optimizing it later. This has the advantage that you can get a bunch of tests working and then use those to validate a later, more complicated, implementation. Arguably Kenneth's suggestion to use conditional assignment is simpler. But it does entail some mods to what I already wrote. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/6] glsl: Add productions to GLSL grammar for switch statement
On 06/20/2011 03:39 PM, Ian Romanick wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/17/2011 05:43 PM, Dan McCabe wrote: The grammar is modified to support switch statements. Rather than follow the grammar in the appendix, which allows case labels to be placed ANYWHERE as a regular statement, we follow the development of the grammar as described in the body of the GLSL. In this variation, the switch statement has a body which consists of a list of case statements. A case statement is preceded by a list of case labels and ends with a list of statements. Either this patch or patch 6/6 is the right place to restrict switch-statements and case labels to GLSL 1.30. From the looks of the series, these patches will merrily allow switch-statements on any GLSL version. That seems bad. :) Isn't that the purpose of the KYEWORD macro in glsl_lexer.ll ? Those macros already specify that that these are valid keywords in 1.3 or beyond already. cheers, danm ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/6] glsl: Create AST structs corresponding to new productions in grammar
On 06/20/2011 03:31 PM, Ian Romanick wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/17/2011 05:43 PM, Dan McCabe wrote: + + exec_list cases; What types are in this list? If this is a list of ast_case_statements, there should be a comment to that effect. We've been really sloppy about that before, but we should change that. :) Sorry, just adhering to the local coding style :). I'll fix that in the next rev. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/6] glsl: Reference data structure ctors in grammar
On 06/20/2011 03:34 PM, Ian Romanick wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/17/2011 05:43 PM, Dan McCabe wrote: We now tie the grammar to the ctors of the ASTs they reference. This requires that we actually have definitions of the ctors. In addition, we also need to define print and hir methods for the AST classes. At this stage of the development, we simply stub out the print and hir methods and flesh them out later. Also, since actual class instances get returned by the productions in the grammar, we also need to designate the type of the productions that reference those instances. + void *ctx = state; + $$ = new(ctx) ast_switch_statement($3, $5); In this instance (and the similar ones below), I'd just do 'new(state)'. After most of the parser was written we started always calling the memory context mem_ctx. Will do. It wasn't clear to me what the rhyme or reason was behind using state vs. ctx. Apparently, my usage matched that rationale. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 6/6] glsl: Generate IR for switch statements
On 06/20/2011 03:50 PM, Ian Romanick wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 06/17/2011 05:43 PM, Dan McCabe wrote:
Beware! Here be dragons!
I think this will generate the wrong code for:
for (i = 0; i 10; i++) {
switch (i) {
default: continue;
}
}
It seems like that will generate some approximation of:
for (i = 0; i 10; i++) {
do {
continue;
break;
} while (true);
}
Right? This is why I had originally tracked loops and switch-statements
in a single stack. In this situation, you'd want to set a do a
continue flag in the switch-statement and emit a break (from the
switch). Something like:
for (i = 0; i 10; i++) {
bool do_cont = false;
do {
do_cont = true;
break;
break;
} while (true);
if (do_cont)
continue;
}
Yikes! Looks like you tripped over one of those dragons :(.
Using a do_cont variable (or similar device) feels kludgey to me. I've
been mulling this over for the last couple of days while I did other
things and I think the right way to do this might be to get rid of the
use of loop altogether.
But the devil is in the details, which I haven't worked out yet. Going
back to the unified loop/switch stack might be needed, though.
cheers, danm
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 6/6] glsl: Generate IR for switch statements
On 06/24/2011 01:17 PM, Dan McCabe wrote:
On 06/20/2011 03:50 PM, Ian Romanick wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 06/17/2011 05:43 PM, Dan McCabe wrote:
Beware! Here be dragons!
I think this will generate the wrong code for:
for (i = 0; i 10; i++) {
switch (i) {
default: continue;
}
}
It seems like that will generate some approximation of:
for (i = 0; i 10; i++) {
do {
continue;
break;
} while (true);
}
Right? This is why I had originally tracked loops and switch-statements
in a single stack. In this situation, you'd want to set a do a
continue flag in the switch-statement and emit a break (from the
switch). Something like:
for (i = 0; i 10; i++) {
bool do_cont = false;
do {
do_cont = true;
break;
break;
} while (true);
if (do_cont)
continue;
}
Yikes! Looks like you tripped over one of those dragons :(.
Using a do_cont variable (or similar device) feels kludgey to me. I've
been mulling this over for the last couple of days while I did other
things and I think the right way to do this might be to get rid of the
use of loop altogether.
But the devil is in the details, which I haven't worked out yet. Going
back to the unified loop/switch stack might be needed, though.
cheers, danm
All is not lost! (But you already knew that :)
One of the motivations of using the loop device is that it enabled the
relatively unmodified infrastructure for break statements. But let's get
rid of the break device and see where we go.
Without xlating switch stmts into loops, we can still retain the
test_val_tmp and is_fallthru_tmp temporaries (in fact, we need to;
nothing changes there since that is how case labels are managed).
However, the standard break processing no longer applies, since we are
not in a loop (for the switch stmt in any event). If we introduce a bool
temporary called is_break_tmp and initialize it to false, then when we
encounter a break stmt, we set is_break_tmp to true. Now we must guard
the case statement with both fallthru_var and is_break_var. If
is_break_tmp was set, we simply clear is_fallthru_tmp right after all
case tests to false.
Note that this approach does require maintaining a loop/switch stack as
the code originally did so that we can tell if a break statement is
associate with a loop or with a switch stmt. Only if the top of the
stack indicates that switch is being processed will the above break
processing be invoked.
Looking at a translation of my canonical example:
switch (expr) {
case c0:
case c1:
stmt0;
case c2:
case c3:
stmt1;
break;
case c4:
default:
stmt2;
}
We can translated this into:
int test_val_tmp = expr;
bool is_fallthru_tmp = false;
bool is_break_tmp = false;
if (test_val_tmp == c0) {
is_fallthru_tmp = true;
}
if (test_val_tmp == c1) {
is_fallthru_tmp = true;
}
if (is_break_tmp) {
is_fallthru_tmp = false;
}
if (is_fallthru_tmp) {
stmt0;
}
if (test_val_tmp == c2) {
is_fallthru_tmp = true;
}
if (test_val_tmp == c3) {
is_fallthru_tmp = true;
}
if (is_break_tmp) {
is_fallthru_tmp = false;
}
if (is_fallthru_tmp) {
stmt1;
is_break_tmp = true; // break stmt
}
if (test_val_tmp == c4) {
is_fallthru_tmp = true;
}
is_fallthru_tmp = true; // default
if (is_break_tmp) {
is_fallthru_tmp = false;
}
if (is_fallthru_tmp) {
stmt2;
}
Comparing this to what we did previously, the things that change in this
code are:
1) no enclosing loop
2) create a bool is_break_tmp initialized to false
3) after all case labels and before each case statement list, clear
is_fallthru_tmp when is_break_tmp
4) set is_break_tmp for each break statement
And maintain the loop/switch stack again as in the original code.
I'll get this cranked out on Monday and resubmit for review. If anyone
has concerns or questions about what I'm doing here, please let me know.
cheers, danm
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/6] glsl: Add support for switch statements
On 06/19/2011 02:21 PM, Eric Anholt wrote: On Sat, 18 Jun 2011 09:29:32 +0200, Keith Packardkei...@keithp.com wrote: On Fri, 17 Jun 2011 17:43:14 -0700, Dan McCabezen3d.li...@gmail.com wrote: break; // implicit exit from loop at end of switch } while (true); Seems like this could just be } while (false); as I don't see any way the loop could go around more than once. while (true) is the instruction available in the hardware, this isn't a description of C code. Correct. It is primarily intended to explain approximately an equivalent for the underlying code and is intended to interpreted with a grain of salt. It is really pseudo-code that looks a lot like C :). I also wanted to use while(true) to explicitly indicate the actual effect of the implicit break at the end of the loop. With while(false), you have to scratch your head to figure out why that final break is there, because it ought not be needed in a while(false) loop. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 0/6] glsl: Add support for switch statements
This patch set adds support for switch statements to the GLSL compiler. We
modify the grammar for the compiler with productions for switch statements
and case labels, while adding supporting supporting productions not already
present. New AST classes are defined to support those productions. However,
with our apporach no new IR is needed, allowing us to leverage all existing
optimizations and code generation.
Regarding the grammar, we note that the grammar as summarized in the appendix
of the GLSL specs leaves a bit to be desired. For example, it appears that case
labels can be used anywhere a statement is valid. However, we note that the
desciption of the switch statement in Section 6.2 in the body of the spec is
much more specific, and we follow that text to guide our creation of new
productions.
Specifically, we add productions for:
switch body,
case label list,
case statement, and
case statement list.
The switch body and the case statement allow us to limit where case labels may
be used.
In turn, we create new AST classes for each of these productions.
As indicated above, our strategy is to not introduce new IR in order to
leverage existing optimizations and code generation. Fortunately, the bread
crumbs that were left in the code from previous work suggested a solution to
processing switch statements that didn't require new IR.
The basic concept for IR creation is that a switch statement generates a loop.
The test expression is evaluated and save in a temporary variable, which is
used for comparing the subsequent case labels. We also manage a fallthru
state that allows us to maintain the sequential semantics of the switch
statement, where cases fall through to the next case in the absence of a break
statement. The loop itself also has an explicit break instruction appended at
the end to force the termination of the loop (the subject of this commit).
Case labels and default cases manipulate the fallthru state. If a case label
equals the test expression, a fall through condition is encountered and the
fallthru state is set to true. Similarly, if a default case is encountered,
the fallthru state is set to true. Note that the fallthru state must be
initialized at the start of the switch statement (at the start of the loop)
to be false. Thereafter, the fallthru state will only ever monotonically
transition to true if a case is matched or if a default case is encountered.
It will never transition from true to false.
The statements associated with each case are then guarded by the fallthru
state. Only if the fallthru state is true do case statements get executed.
We will illustrate the analogous loop and conditional code that a switch
statement corresponds to by examining an example.
Consider the following switch statement:
switch (42) {
case 0:
case 1:
gl_FragColor = vec4(1.0, 2.0, 3.0, 4.0);
case 2:
case 3:
gl_FragColor = vec4(4.0, 3.0, 2.0, 1.0);
break;
case 4:
default:
gl_FragColor = vec4(0.0, 0.0, 0.0, 0.0);
}
Note that case 0 and case 1 fall through to cases 2 and 3 if they occur.
Note that case 4 and the default case must be reached explicitly, since cases
2 and 3 break at the end of their case.
Finally, note that case 4 and the default case don't break but simply fall
through to the end of the switch.
For this code, the equivalent code can be expressed as:
do {
int test = 42; // capture test expression
bool fallthru = false; // inhibit initial fall throughs
if (test == 0) // case 0
fallthru = true;
if (test == 1) // case 1
fallthru = true;
if (fallthru) {
gl_FragColor = vec4(1.0, 2.0, 3.0, 4.0);
}
if (test == 2) // case 2
fallthru = true;
if (test == 3) // case 3
fallthru = true;
if (fallthru) {
gl_FragColor = vec4(4.0, 3.0, 2.0, 1.0);
break; // most AST/IR processing previously in place
}
if (test == 4) // case 4
fallthru = true;
fallthru = true; // default case
if (fallthru) {
gl_FragColor = vec4(0.0, 0.0, 0.0, 0.0);
}
break; // implicit exit from loop at end of switch
} while (true);
Although we expressed our transformation into a do/while loop, we could
have used any other loop structure to explain the concept. However, we do
want to point out the existance of the last break statement which gets
implicitly generated to force loop termination at the end of the switch
statement.
___
mesa-dev mailing list
[Mesa-dev] [PATCH 1/6] glsl: Create AST data structures for switch statement and case label
Data structures for switch statement and case label are created that parallel
the structure of other AST data.
---
src/glsl/ast.h | 24
1 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/src/glsl/ast.h b/src/glsl/ast.h
index 878f48b..39a59d4 100644
--- a/src/glsl/ast.h
+++ b/src/glsl/ast.h
@@ -628,13 +628,19 @@ public:
class ast_case_label : public ast_node {
public:
+ ast_case_label(ast_expression *test_value);
+ virtual void print(void) const;
+
+ virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
/**
-* An expression of NULL means 'default'.
+* An test value of NULL means 'default'.
*/
- ast_expression *expression;
+ ast_expression *test_value;
};
+
class ast_selection_statement : public ast_node {
public:
ast_selection_statement(ast_expression *condition,
@@ -653,8 +659,18 @@ public:
class ast_switch_statement : public ast_node {
public:
- ast_expression *expression;
- exec_list statements;
+ ast_switch_statement(ast_expression *test_expression,
+ ast_node *body);
+ virtual void print(void) const;
+
+ virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
+
+ ast_expression *test_expression;
+ ast_node *body;
+
+protected:
+ void test_to_hir(class ir_loop *, struct _mesa_glsl_parse_state *);
};
class ast_iteration_statement : public ast_node {
--
1.7.4.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/6] glsl: Add productions to GLSL grammar for switch statement
The grammar is modified to support switch statements. Rather than follow the
grammar in the appendix, which allows case labels to be placed ANYWHERE
as a regular statement, we follow the development of the grammar as
described in the body of the GLSL.
In this variation, the switch statement has a body which consists of a list
of case statements. A case statement is preceded by a list of case labels and
ends with a list of statements.
---
src/glsl/glsl_parser.yy | 64 --
1 files changed, 61 insertions(+), 3 deletions(-)
diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
index 2c0498e..b3727ce 100644
--- a/src/glsl/glsl_parser.yy
+++ b/src/glsl/glsl_parser.yy
@@ -206,6 +206,12 @@
%type declaration struct_declarator_list
%type node selection_statement
%type selection_rest_statement selection_rest_statement
+%type node switch_statement
+%type node switch_body
+%type node case_label
+%type node case_label_list
+%type node case_statement
+%type node case_statement_list
%type node iteration_statement
%type node condition
%type node conditionopt
@@ -1519,8 +1525,7 @@ simple_statement:
declaration_statement
| expression_statement
| selection_statement
- | switch_statement { $$ = NULL; }
- | case_label{ $$ = NULL; }
+ | switch_statement
| iteration_statement
| jump_statement
;
@@ -1642,15 +1647,68 @@ condition:
}
;
+/*
+ * siwtch_statement grammar is based on the syntax described in the body
+ * of the GLSL spec, not in it's appendix!!!
+ */
switch_statement:
- SWITCH '(' expression ')' compound_statement
+ SWITCH '(' expression ')' switch_body
+ {
+ $$ = NULL;
+ }
+ ;
+
+switch_body:
+ '{' '}'
+ {
+ $$ = NULL;
+ }
+ | '{' case_statement_list '}'
+ {
+ $$ = NULL;
+ }
;
case_label:
CASE expression ':'
+ {
+ $$ = NULL;
+ }
| DEFAULT ':'
+ {
+ $$ = NULL;
+ }
;
+case_label_list:
+ case_label
+ {
+ $$ = NULL;
+ }
+ | case_label_list case_label
+ {
+ $$ = NULL;
+ }
+ ;
+
+case_statement:
+ case_label_list statement_list
+ {
+ $$ = NULL;
+ }
+ ;
+
+case_statement_list:
+ case_statement
+ {
+ $$ = NULL;
+ }
+ | case_statement_list case_statement
+ {
+ $$ = NULL;
+ }
+ ;
+
iteration_statement:
WHILE '(' condition ')' statement_no_new_scope
{
--
1.7.4.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/6] glsl: Create AST structs corresponding to new productions in grammar
Previously we added productions for:
switch_body
case_label_list
case_statement
case_statement_list
Now add AST structs corresponding to those productions.
---
src/glsl/ast.h | 49 +
1 files changed, 49 insertions(+), 0 deletions(-)
diff --git a/src/glsl/ast.h b/src/glsl/ast.h
index 39a59d4..b0222a8 100644
--- a/src/glsl/ast.h
+++ b/src/glsl/ast.h
@@ -641,6 +641,55 @@ public:
};
+class ast_case_label_list : public ast_node {
+public:
+ ast_case_label_list(void);
+ virtual void print(void) const;
+
+ virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
+
+ exec_list labels;
+};
+
+
+class ast_case_statement : public ast_node {
+public:
+ ast_case_statement(ast_case_label_list *labels);
+ virtual void print(void) const;
+
+ virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
+
+ ast_case_label_list *labels;
+ exec_list stmts;
+};
+
+
+class ast_case_statement_list : public ast_node {
+public:
+ ast_case_statement_list(void);
+ virtual void print(void) const;
+
+ virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
+
+ exec_list cases;
+};
+
+
+class ast_switch_body : public ast_node {
+public:
+ ast_switch_body(ast_case_statement_list *stmts);
+ virtual void print(void) const;
+
+ virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
+
+ ast_case_statement_list *stmts;
+};
+
+
class ast_selection_statement : public ast_node {
public:
ast_selection_statement(ast_expression *condition,
--
1.7.4.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/6] glsl: Reference data structure ctors in grammar
We now tie the grammar to the ctors of the ASTs they reference.
This requires that we actually have definitions of the ctors.
In addition, we also need to define print and hir methods for the AST
classes. At this stage of the development, we simply stub out the print
and hir methods and flesh them out later.
Also, since actual class instances get returned by the productions in the
grammar, we also need to designate the type of the productions that
reference those instances.
---
src/glsl/ast_to_hir.cpp | 54
src/glsl/glsl_parser.yy | 58 ++
src/glsl/glsl_parser_extras.cpp | 74 +++
3 files changed, 170 insertions(+), 16 deletions(-)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 3b87f0d..553e459 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -3274,6 +3274,60 @@ ast_selection_statement::hir(exec_list *instructions,
}
+ir_rvalue *
+ast_switch_statement::hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state)
+{
+ // TODO - implement me!!!
+ return NULL;
+}
+
+
+ir_rvalue *
+ast_switch_body::hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state)
+{
+ // TODO - implement me!!!
+ return NULL;
+}
+
+
+ir_rvalue *
+ast_case_statement::hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state)
+{
+ // TODO - implement me!!!
+ return NULL;
+}
+
+
+ir_rvalue *
+ast_case_statement_list::hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state)
+{
+ // TODO - implement me!!!
+ return NULL;
+}
+
+
+ir_rvalue *
+ast_case_label::hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state)
+{
+ // TODO - implement me!!!
+ return NULL;
+}
+
+
+ir_rvalue *
+ast_case_label_list::hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state)
+{
+ // TODO - implement me!!!
+ return NULL;
+}
+
+
void
ast_iteration_statement::condition_to_hir(ir_loop *stmt,
struct _mesa_glsl_parse_state *state)
diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
index b3727ce..3ba3282 100644
--- a/src/glsl/glsl_parser.yy
+++ b/src/glsl/glsl_parser.yy
@@ -67,6 +67,11 @@
ast_declarator_list *declarator_list;
ast_struct_specifier *struct_specifier;
ast_declaration *declaration;
+ ast_switch_body *switch_body;
+ ast_case_label *case_label;
+ ast_case_label_list *case_label_list;
+ ast_case_statement *case_statement;
+ ast_case_statement_list *case_statement_list;
struct {
ast_node *cond;
@@ -207,11 +212,11 @@
%type node selection_statement
%type selection_rest_statement selection_rest_statement
%type node switch_statement
-%type node switch_body
-%type node case_label
-%type node case_label_list
-%type node case_statement
-%type node case_statement_list
+%type switch_body switch_body
+%type case_label_list case_label_list
+%type case_label case_label
+%type case_statement case_statement
+%type case_statement_list case_statement_list
%type node iteration_statement
%type node condition
%type node conditionopt
@@ -1654,58 +1659,79 @@ condition:
switch_statement:
SWITCH '(' expression ')' switch_body
{
- $$ = NULL;
+ void *ctx = state;
+ $$ = new(ctx) ast_switch_statement($3, $5);
}
;
switch_body:
'{' '}'
{
- $$ = NULL;
+ void *ctx = state;
+ $$ = new(ctx) ast_switch_body(NULL);
+ $$-set_location(yylloc);
}
| '{' case_statement_list '}'
{
- $$ = NULL;
+ void *ctx = state;
+ $$ = new(ctx) ast_switch_body($2);
+ $$-set_location(yylloc);
}
;
case_label:
CASE expression ':'
{
- $$ = NULL;
+ $$ = new(state) ast_case_label($2);
}
| DEFAULT ':'
{
- $$ = NULL;
+ $$ = new(state) ast_case_label(NULL);
}
;
case_label_list:
case_label
{
- $$ = NULL;
+ ast_case_label_list *labels = new(state) ast_case_label_list();
+
+ labels-labels.push_tail( $1-link);
+ $$ = labels;
}
| case_label_list case_label
{
- $$ = NULL;
+ $$ = $1;
+ $$-labels.push_tail( $2-link);
}
;
case_statement:
- case_label_list statement_list
+ case_label_list statement
{
- $$ = NULL;
+ ast_case_statement *stmts = new(state) ast_case_statement($1);
+
+ stmts-stmts.push_tail( $2-link);
+ $$ = stmts
+ }
+ | case_statement statement
+ {
+ $$ = $1;
+ $$-stmts.push_tail( $2-link);
}
;
case_statement_list:
[Mesa-dev] [PATCH 6/6] glsl: Generate IR for switch statements
Beware! Here be dragons!
Up until now modyfing the GLSL compiler has been pretty straightforward.
This is where things get interesting.
Switch statement processing leverages infrastructure that was previously
created (specifically for break statements, which are encountered in both
loops and switch statements). Rather than generating new IR constructs,
which also requires creating new code generation and optimization, we take
advantage of the existing infrastructure and IR. Fortunately, the bread
crumbs that were left in the code from previous work suggested a solution
to processing switch statements.
The basic concept is that a switch statement generates a loop. The test
expression is evaluated and saved in a temporary variable, which is used
for comparing the subsequent case labels. We also manage a fallthru state
that allows us to maintain the sequential semantics of the switch statement,
where cases fall through to the next case in the absence of a break statement.
The loop itself also has an explicit break instruction appended at the end
to force the termination of the loop (the subject of this commit).
Case labels and default cases manipulate the fallthru state. If a case label
equals the test expression, a fall through condition is encountered and the
fallthru state is set to true. Similarly, if a default case is encountered,
the fallthru state is set to true. Note that the fallthru state must be
initialized at the start of the switch statement (at the start of the loop)
to be false. Thereafter, the fallthru state will only ever monotonically
transition to true if a case is matched or if a default case is encountered.
It will never transition from true to false.
The statements associated with each case are then guarded by the fallthru
state. Only if the fallthru state is true do case statements get executed.
We will illustrate the analogous loop and conditional code that a switch
statement corresponds to by examining an example.
Consider the following switch statement:
switch (42) {
case 0:
case 1:
gl_FragColor = vec4(1.0, 2.0, 3.0, 4.0);
case 2:
case 3:
gl_FragColor = vec4(4.0, 3.0, 2.0, 1.0);
break;
case 4:
default:
gl_FragColor = vec4(0.0, 0.0, 0.0, 0.0);
}
Note that case 0 and case 1 fall through to cases 2 and 3 if they occur.
Note that case 4 and the default case must be reached explicitly, since cases
2 and 3 break at the end of their case.
Finally, note that case 4 and the default case don't break but simply fall
through to the end of the switch.
For this code, the equivalent code can be expressed as:
do {
int test = 42; // capture test expression
bool fallthru = false; // prevent initial fall throughs
if (test == 0) // case 0
fallthru = true;
if (test == 1) // case 1
fallthru = true;
if (fallthru) {
gl_FragColor = vec4(1.0, 2.0, 3.0, 4.0);
}
if (test == 2) // case 2
fallthru = true;
if (test == 3) // case 3
fallthru = true;
if (fallthru) {
gl_FragColor = vec4(4.0, 3.0, 2.0, 1.0);
break; // most AST/IR processing previously in place
}
if (test == 4) // case 4
fallthru = true;
fallthru = true; // default case
if (fallthru) {
gl_FragColor = vec4(0.0, 0.0, 0.0, 0.0);
}
break; // implicit exit from loop at end of switch
} while (true);
Although we expressed our transformation into a do/while loop, we could
have used any other loop structure to explain the concept. However, we do
want to point out the existance of the last break statement which gets
implicitly generated to force loop termination at the end of the switch
statement.
---
src/glsl/ast_to_hir.cpp | 237 +++
src/glsl/glsl_parser_extras.cpp |3 +-
src/glsl/glsl_parser_extras.h |8 +-
3 files changed, 198 insertions(+), 50 deletions(-)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 553e459..628bbfc 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -3184,39 +3184,37 @@ ast_jump_statement::hir(exec_list *instructions,
case ast_break:
case ast_continue:
- /* FINISHME: Handle switch-statements. They cannot contain 'continue',
- * FINISHME: and they use a different IR instruction for 'break'.
- */
- /* FINISHME: Correctly handle the nesting. If a switch-statement is
- * FINISHME: inside a loop, a 'continue' is valid and will bind to the
- * FINISHME: loop.
- */
- if
[Mesa-dev] [PATCH 2/7] Add productions to GLSL grammar for switch statement
The grammar is modified to support switch statements. Rather than follow the
grammar in the appendix, which allows case labels to be placed ANYWHERE
as a regular statement, we follow the development of the grammar as
described in the body of the GLSL.
In this variation, the switch statement has a body which consists of a list
of case statements. A case statement is preceded by a list of case labels and
ends with a list of statements.
---
src/glsl/glsl_parser.yy | 64 --
1 files changed, 61 insertions(+), 3 deletions(-)
diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
index 2c0498e..b3727ce 100644
--- a/src/glsl/glsl_parser.yy
+++ b/src/glsl/glsl_parser.yy
@@ -206,6 +206,12 @@
%type declaration struct_declarator_list
%type node selection_statement
%type selection_rest_statement selection_rest_statement
+%type node switch_statement
+%type node switch_body
+%type node case_label
+%type node case_label_list
+%type node case_statement
+%type node case_statement_list
%type node iteration_statement
%type node condition
%type node conditionopt
@@ -1519,8 +1525,7 @@ simple_statement:
declaration_statement
| expression_statement
| selection_statement
- | switch_statement { $$ = NULL; }
- | case_label{ $$ = NULL; }
+ | switch_statement
| iteration_statement
| jump_statement
;
@@ -1642,15 +1647,68 @@ condition:
}
;
+/*
+ * siwtch_statement grammar is based on the syntax described in the body
+ * of the GLSL spec, not in it's appendix!!!
+ */
switch_statement:
- SWITCH '(' expression ')' compound_statement
+ SWITCH '(' expression ')' switch_body
+ {
+ $$ = NULL;
+ }
+ ;
+
+switch_body:
+ '{' '}'
+ {
+ $$ = NULL;
+ }
+ | '{' case_statement_list '}'
+ {
+ $$ = NULL;
+ }
;
case_label:
CASE expression ':'
+ {
+ $$ = NULL;
+ }
| DEFAULT ':'
+ {
+ $$ = NULL;
+ }
;
+case_label_list:
+ case_label
+ {
+ $$ = NULL;
+ }
+ | case_label_list case_label
+ {
+ $$ = NULL;
+ }
+ ;
+
+case_statement:
+ case_label_list statement_list
+ {
+ $$ = NULL;
+ }
+ ;
+
+case_statement_list:
+ case_statement
+ {
+ $$ = NULL;
+ }
+ | case_statement_list case_statement
+ {
+ $$ = NULL;
+ }
+ ;
+
iteration_statement:
WHILE '(' condition ')' statement_no_new_scope
{
--
1.7.4.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/7] Create AST structs corresping to new productions in grammar
Previously we added productions for:
switch_body
case_label_list
case_statement
case_statement_list
Now add AST structs corresponding to those productions.
---
src/glsl/ast.h | 49 +
1 files changed, 49 insertions(+), 0 deletions(-)
diff --git a/src/glsl/ast.h b/src/glsl/ast.h
index 48d1795..626a8df 100644
--- a/src/glsl/ast.h
+++ b/src/glsl/ast.h
@@ -641,6 +641,55 @@ public:
};
+class ast_case_label_list : public ast_node {
+public:
+ ast_case_label_list(void);
+ virtual void print(void) const;
+
+ virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
+
+ exec_list labels;
+};
+
+
+class ast_case_statement : public ast_node {
+public:
+ ast_case_statement(ast_case_label_list *labels);
+ virtual void print(void) const;
+
+ virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
+
+ ast_case_label_list *labels;
+ exec_list stmts;
+};
+
+
+class ast_case_statement_list : public ast_node {
+public:
+ ast_case_statement_list(void);
+ virtual void print(void) const;
+
+ virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
+
+ exec_list cases;
+};
+
+
+class ast_switch_body : public ast_node {
+public:
+ ast_switch_body(ast_case_statement_list *stmts);
+ virtual void print(void) const;
+
+ virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
+
+ ast_case_statement_list *stmts;
+};
+
+
class ast_selection_statement : public ast_node {
public:
ast_selection_statement(ast_expression *condition,
--
1.7.4.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/7] Reference data structure ctors in grammar
We now tie the grammar to the ctors of the ASTs they reference.
This requires that we actually have definitions of the ctors.
In addition, we also need to define print and hir methods for the AST
classes. At this stage of the development, we simply stub out the print
and hir methods and flesh them out later.
Also, since actual class instances get returned by the productions in the
grammar, we also need to designate the type of the productions that
reference those instances.
---
src/glsl/ast_to_hir.cpp | 54
src/glsl/glsl_parser.yy | 58 ++
src/glsl/glsl_parser_extras.cpp | 74 +++
3 files changed, 170 insertions(+), 16 deletions(-)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 3b87f0d..553e459 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -3274,6 +3274,60 @@ ast_selection_statement::hir(exec_list *instructions,
}
+ir_rvalue *
+ast_switch_statement::hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state)
+{
+ // TODO - implement me!!!
+ return NULL;
+}
+
+
+ir_rvalue *
+ast_switch_body::hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state)
+{
+ // TODO - implement me!!!
+ return NULL;
+}
+
+
+ir_rvalue *
+ast_case_statement::hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state)
+{
+ // TODO - implement me!!!
+ return NULL;
+}
+
+
+ir_rvalue *
+ast_case_statement_list::hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state)
+{
+ // TODO - implement me!!!
+ return NULL;
+}
+
+
+ir_rvalue *
+ast_case_label::hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state)
+{
+ // TODO - implement me!!!
+ return NULL;
+}
+
+
+ir_rvalue *
+ast_case_label_list::hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state)
+{
+ // TODO - implement me!!!
+ return NULL;
+}
+
+
void
ast_iteration_statement::condition_to_hir(ir_loop *stmt,
struct _mesa_glsl_parse_state *state)
diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
index b3727ce..3ba3282 100644
--- a/src/glsl/glsl_parser.yy
+++ b/src/glsl/glsl_parser.yy
@@ -67,6 +67,11 @@
ast_declarator_list *declarator_list;
ast_struct_specifier *struct_specifier;
ast_declaration *declaration;
+ ast_switch_body *switch_body;
+ ast_case_label *case_label;
+ ast_case_label_list *case_label_list;
+ ast_case_statement *case_statement;
+ ast_case_statement_list *case_statement_list;
struct {
ast_node *cond;
@@ -207,11 +212,11 @@
%type node selection_statement
%type selection_rest_statement selection_rest_statement
%type node switch_statement
-%type node switch_body
-%type node case_label
-%type node case_label_list
-%type node case_statement
-%type node case_statement_list
+%type switch_body switch_body
+%type case_label_list case_label_list
+%type case_label case_label
+%type case_statement case_statement
+%type case_statement_list case_statement_list
%type node iteration_statement
%type node condition
%type node conditionopt
@@ -1654,58 +1659,79 @@ condition:
switch_statement:
SWITCH '(' expression ')' switch_body
{
- $$ = NULL;
+ void *ctx = state;
+ $$ = new(ctx) ast_switch_statement($3, $5);
}
;
switch_body:
'{' '}'
{
- $$ = NULL;
+ void *ctx = state;
+ $$ = new(ctx) ast_switch_body(NULL);
+ $$-set_location(yylloc);
}
| '{' case_statement_list '}'
{
- $$ = NULL;
+ void *ctx = state;
+ $$ = new(ctx) ast_switch_body($2);
+ $$-set_location(yylloc);
}
;
case_label:
CASE expression ':'
{
- $$ = NULL;
+ $$ = new(state) ast_case_label($2);
}
| DEFAULT ':'
{
- $$ = NULL;
+ $$ = new(state) ast_case_label(NULL);
}
;
case_label_list:
case_label
{
- $$ = NULL;
+ ast_case_label_list *labels = new(state) ast_case_label_list();
+
+ labels-labels.push_tail( $1-link);
+ $$ = labels;
}
| case_label_list case_label
{
- $$ = NULL;
+ $$ = $1;
+ $$-labels.push_tail( $2-link);
}
;
case_statement:
- case_label_list statement_list
+ case_label_list statement
{
- $$ = NULL;
+ ast_case_statement *stmts = new(state) ast_case_statement($1);
+
+ stmts-stmts.push_tail( $2-link);
+ $$ = stmts
+ }
+ | case_statement statement
+ {
+ $$ = $1;
+ $$-stmts.push_tail( $2-link);
}
;
case_statement_list:
[Mesa-dev] [PATCH 5/7] Flesh out AST print methods
Pretty trivial stuff. Simply print the structure of the ASTs. No magic here.
---
src/glsl/glsl_parser_extras.cpp | 38 --
1 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
index a351e12..f2ed518 100644
--- a/src/glsl/glsl_parser_extras.cpp
+++ b/src/glsl/glsl_parser_extras.cpp
@@ -695,7 +695,11 @@
ast_selection_statement::ast_selection_statement(ast_expression *condition,
void
ast_switch_statement::print(void) const
{
- printf(TODO - implement me!!!);
+ printf(switch ( );
+ test_expression-print();
+ printf() );
+
+ body-print();
}
@@ -710,7 +714,11 @@ ast_switch_statement::ast_switch_statement(ast_expression
*test_expression,
void
ast_switch_body::print(void) const
{
- printf(TODO - implement me!!!);
+ printf({\n);
+ if (stmts != NULL) {
+ stmts-print();
+ }
+ printf(}\n);
}
@@ -722,7 +730,13 @@ ast_switch_body::ast_switch_body(ast_case_statement_list
*stmts)
void ast_case_label::print(void) const
{
- printf(TODO - implement me!!!);
+ if (test_value != NULL) {
+ printf(case );
+ test_value-print();
+ printf(: );
+ } else {
+ printf(default: );
+ }
}
@@ -734,7 +748,11 @@ ast_case_label::ast_case_label(ast_expression *test_value)
void ast_case_label_list::print(void) const
{
- printf(TODO - implement me!!!);
+ foreach_list_const(n, this-labels) {
+ ast_node *ast = exec_node_data(ast_node, n, link);
+ ast-print();
+ }
+ printf(\n);
}
@@ -745,7 +763,12 @@ ast_case_label_list::ast_case_label_list(void)
void ast_case_statement::print(void) const
{
- printf(TODO - implement me!!!);
+ labels-print();
+ foreach_list_const(n, this-stmts) {
+ ast_node *ast = exec_node_data(ast_node, n, link);
+ ast-print();
+ printf(\n);
+ }
}
@@ -757,7 +780,10 @@ ast_case_statement::ast_case_statement(ast_case_label_list
*labels)
void ast_case_statement_list::print(void) const
{
- printf(TODO - implement me!!!);
+ foreach_list_const(n, this-cases) {
+ ast_node *ast = exec_node_data(ast_node, n, link);
+ ast-print();
+ }
}
--
1.7.4.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 6/7] Generate IR for switch statements
Beware! Here be dragons!
Up until now modyfing the GLSL compiler has been pretty straightforward.
This is where things get interesting.
Switch statement processing leverages infrastructure that was previously
created (specifically for break statements, which are encountered in both
loops and switch statements). Rather than generating new IR constructs,
which also requires creating new code generation and optimization, we take
advantage of the existing infrastructure and IR. Fortunately, the bread
crumbs that were left in the code from previous work suggested a solution
to processing switch statements.
The basic concept is that a switch statement generates a loop. The test
expression is evaluated and save in a temporary variable, which is used
for comparing the subsequent case labels. We also manage a fallthru state
that allows us to maintain the sequential semantics of the switch statement,
where cases fall through to the next case in the absence of a break statement.
The loop itself also has an explicit break instruction appended at the end
to force the termination of the loop (the subject of this commit).
Case labels and default cases manipulate the fallthru state. If a case label
equals the test expression, a fall through condition is encountered and the
fallthru state is set to true. Similarly, if a default case is encountered,
the fallthru state is set to true. Note that the fallthru state must be
initialized at the start of the switch statement (at the start of the loop)
to be false. Thereafter, the fallthru state will only ever monotonically
transition to true if a case is matched or if a default case is encountered.
It will never transition from true to false.
The statements associated with each case are then guarded by the fallthru
state. Only if the fallthru state is true do case statements get executed.
We will illustrate the analogous loop and conditional code that a switch
statement corresponds to by examining an example.
Consider the following switch statement:
switch (42) {
case 0:
case 1:
gl_FragColor = vec4(1.0, 2.0, 3.0, 4.0);
case 2:
case 3:
gl_FragColor = vec4(4.0, 3.0, 2.0, 1.0);
break;
case 4:
default:
gl_FragColor = vec4(0.0, 0.0, 0.0, 0.0);
}
Note that case 0 and case 1 fall through to cases 2 and 3 if they occur.
Note that case 4 and the default case must be reached explicitly, since cases
2 and 3 break at the end of their case.
Finally, note that case 4 and the default case don't break but simply fall
through to the end of the switch.
For this code, the equivalent code can be expressed as:
do {
int test = 42; // capture test expression
bool fallthru = false; // prevent initial fall throughs
if (test == 0) // case 0
fallthru = true;
if (test == 1) // case 1
fallthru = true;
if (fallthru) {
gl_FragColor = vec4(1.0, 2.0, 3.0, 4.0);
}
if (test == 2) // case 2
fallthru = true;
if (test == 3) // case 3
fallthru = true;
if (fallthru) {
gl_FragColor = vec4(4.0, 3.0, 2.0, 1.0);
break; // most AST/IR processing previously in place
}
if (test == 4) // case 4
fallthru = true;
fallthru = true; // default case
if (fallthru) {
gl_FragColor = vec4(0.0, 0.0, 0.0, 0.0);
}
break; // implicit exit from loop at end of switch
} while (true);
Although we expressed our transformation into a do/while loop, we could
have used any other loop structure to explain the concept. However, we do
want to point out the existance of the last break statement which gets
implicitly generated to force loop termination at the end of the switch
statement.
---
src/glsl/ast_to_hir.cpp | 227 +++
src/glsl/glsl_parser_extras.cpp |3 +-
src/glsl/glsl_parser_extras.h |4 +-
3 files changed, 184 insertions(+), 50 deletions(-)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 553e459..c409c2d 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -3184,39 +3184,37 @@ ast_jump_statement::hir(exec_list *instructions,
case ast_break:
case ast_continue:
- /* FINISHME: Handle switch-statements. They cannot contain 'continue',
- * FINISHME: and they use a different IR instruction for 'break'.
- */
- /* FINISHME: Correctly handle the nesting. If a switch-statement is
- * FINISHME: inside a loop, a 'continue' is valid and will bind to the
- * FINISHME: loop.
- */
- if
[Mesa-dev] Error building glxext.c
Hi all, Apologies if this is a stupid n00b question, but I'm getting errors building glxext.c in mesa/mesa/src/glx. Specific error msg is: glxext.c: In function ‘__glXWireToEvent’: glxext.c:141:35: error: ‘xGLXBufferSwapComplete’ has no member named ‘sbc_hi’ glxext.c:141:58: error: ‘xGLXBufferSwapComplete’ has no member named ‘sbc_lo’ I followed the installation/build directions described here: http://wiki.x.org/wiki/ModularDevelopersGuide and I grabbed the src from git last night (early evening). Any suggestions/recommendations? TIA cheers, danm ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] Error building glxext.c
Hi all, Apologies if this is a stupid n00b question, but I'm getting errors building glxext.c in mesa/mesa/src/glx. Specific error msg is: glxext.c: In function ‘__glXWireToEvent’: glxext.c:141:35: error: ‘xGLXBufferSwapComplete’ has no member named ‘sbc_hi’ glxext.c:141:58: error: ‘xGLXBufferSwapComplete’ has no member named ‘sbc_lo’ I followed the installation/build directions described here: http://wiki.x.org/wiki/ModularDevelopersGuide and I grabbed the src from git last night (early evening). Any suggestions/recommendations? TIA cheers, danm ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
