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 McCabe 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.
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 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.
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 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 Anholt 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] 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 Versace 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 Anholt Signed-off-by: Chad Versace --- 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] Implementing varying packing
On 08/16/2011 07:06 AM, vlj wrote: From: Vincent Lejeune 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 McCabe 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] glsl: Fix type error when lowering integer divisions
On 08/15/2011 09:48 AM, Paul Berry wrote: On 15 August 2011 08:50, Jose Fonseca wrote: In places you don't have native int division support, you could use one Newton-Raphson iteration step for almost accurate results, assuggested accuracy of SSE2's RCPPS instructions. See for reference the following llvmpipe comment: /** * Do one Newton-Raphson step to improve reciprocate precision: * * x_{i+1} = x_i * (2 - a * x_i) * * XXX: Unfortunately this won't give IEEE-754 conformant results for 0 or * +/-Inf, giving NaN instead. Certain applications rely on this behavior, * such as Google Earth, which does RCP(RSQRT(0.0) when drawing the Earth's * halo. It would be necessary to clamp the argument to prevent this. * * See also: * - http://en.wikipedia.org/wiki/Division_(digital)#Newton.E2.80.93Raphson_division * - http://softwarecommunity.intel.com/articles/eng/1818.htm */ The softwarecommunity.intel.com link is down, but the "Intel® 64 and IA-32 Architectures Optimization Reference Manual" also documents this. As mentioned, the N-R iteration gives wrong results for the reciprocate of +/-inf, but that's guaranteed to never happen when the arguments are integers encoded as floats. Jose Thanks for the reference, Jose. My understanding is that applying Newton-Raphson to the reciprocal operation won't help directly in this case (since the problem is due to the fundamental precision limit of 32-bit floats, and happens even if the reciprocal is computed perfectly), but we may be able to cook up a variation on N-R that works in this case. Another idea I've been toying around with would be to implement the equivalent of the following GLSL: int divide(int x, int y) { int quotient = int(float(x)*reciprocal(float(y))); if ((quotient+1)*y == x) { // Fix the case where y divides x exactly, and rounding errors cause // us to compute the wrong quotient. quotient = quotient + 1; } return quotient; } (the actual routine would have to be slightly more complex than this to make sure to do the right thing for negative values). It's kind of an ugly hack, but it wouldn't cost too many GPU instructions, and it would give us sufficient accuracy for pre-GLSL 1.30. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev 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). cheers, danm ___ 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 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 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
Re: [Mesa-dev] S2TC - yet another attempt to solve the "S3TC issue"
On 08/03/2011 12:47 PM, Ian Romanick wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 08/03/2011 12:11 PM, Bryan Cain wrote: On 08/03/2011 01:58 PM, Ian Romanick wrote: I think this solves the issue for the compressor and for the software decompressor. I don't think this solves the problem for the decompressor for hardware drivers, so that may still pose a problem for Linux distros. Pardon my ignorance, but why do hardware drivers need a decompressor? To quote the EXT_texture_compression_s3tc spec: WARNING: Vendors able to support S3TC texture compression in Direct3D drivers do not necessarily have the right to use the same functionality in OpenGL. This is the same issue that Linux distros have with ARB_texture_float being enabled in hardware drivers. The hardware may implement the functionality, and the hardware vendor may have some license for the patent, but that license may not cover making the functionality available in Mesa. That S3 has sued Apple is some indication that these licenses may have very narrow scope. Not being a lawyer (nor even playing one on television), all this is doing is feeding data in a form the hardware can interpret. The algorithms used in the creation of the data in this case bears no resemblance to any algorithms that create "high quality" data that the hardware, which was created under license from S3, might consume. You could argue that using the next courser MIP level (2x2 subsampling with low-pass filtering) in an RGB565 format with dithering to get a virtual 32-bit color space accomplishes a similar effect with a comparable decrease in the memory footprint, since S3TC uses 2 16-bit colors and a 4x4 matrix of 2-bit fields per 4x4 collection of texels. This shouldn't infringe on S3's S3TC compression and/or decompression algorithms (although I cannot comment on whether it infringes on any other patents). YMMV. If you want a definitive answer, you might want to consult an attorney who is willing to defend you in the event of infringement. Otherwise, all bets are off. But if I were a gambling man (and weren't employed by a company that may not want to place such a bet without appropriate legal advice), ... Ultimately, this a legal, not a technological, question. cheers, danm Disclaimer: I wrote the original reference implementation of S3TC as well as the Game Developer article with John Brothers that described S3's technology while an employee of S3. I no longer work for S3. Also, I am not a lawyer (see my first point in the reply). -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAk45peMACgkQX1gOwKyEAw9p+ACgiCiOZLUcLM4ItXGtCKWvlRTl +wMAnA3wUg5WcXsgwsZZQnq+iCT2Rr7h =6myP -END PGP SIGNATURE- ___ 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
[Mesa-dev] [PATCH 5/5] glsl: Generate IR for switch statements
Up until now modifying the GLSL compiler has been pretty straightforward. This is where things get interesting. But still pretty straightforward. 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 fallthru on previous break if (is_fallthru) { gl_FragColor = vec4(0.0, 0.0, 0.0, 0.0); } The code generate for |= and &= uses the conditional assignment capabilities of the IR. --- src/glsl/ast_to_hir.cpp | 286 +-- src/glsl/glsl_parser_extras.cpp |3 +- src/glsl/glsl_parser_extras.h |
[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 c0524bf..db5b4ad 100644 --- a/src/glsl/ast_to_hir.cpp +++ b/src/glsl/ast_to_hir.cpp @@ -3319,6 +3319,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 selection_statement %type selection_rest_statement %type switch_statement -%type switch_body -%type case_label -%type case_label_list -%type case_statement -%type case_statement_list +%type switch_body +%type case_label_list +%type case_label +%type case_statement +%type case_statement_list %type iteration_statement %type condition %type 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(); + + cases->cases.push_tail(& $1->link); + $$ = cases; } | case_statement_list ca
[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 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 struct_declarator_list %type selection_statement %type selection_rest_statement +%type switch_statement +%type switch_body +%type case_label +%type case_label_list +%type case_statement +%type case_statement_list %type iteration_statement %type condition %type 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 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 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
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 Versace --- 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.
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 Versace --- 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
[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 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
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 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
[Mesa-dev] [PATCH 5/5] glsl: Generate IR for switch statements
Up until now modifying the GLSL compiler has been pretty straightforward. This is where things get interesting. But still pretty straightforward. 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 fallthru on previous break if (is_fallthru) { gl_FragColor = vec4(0.0, 0.0, 0.0, 0.0); } The code generate for |= and &= uses the conditional assignment capabilities of the IR. LocalWords: glsl fallthru gl FragColor vec stmt unstage src Untracked yy cpp --- src/glsl/ast_to_hir.cpp | 286 +
[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 selection_statement %type selection_rest_statement %type switch_statement -%type switch_body -%type case_label -%type case_label_list -%type case_statement -%type case_statement_list +%type switch_body +%type case_label_list +%type case_label +%type case_statement +%type case_statement_list %type iteration_statement %type condition %type 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(); + + cases->cases.push_tail(& $1->link); + $$ = cases; } | case_statement_list ca
[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 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 struct_declarator_list %type selection_statement %type selection_rest_statement +%type switch_statement +%type switch_body +%type case_label +%type case_label_list +%type case_statement +%type case_statement_list %type iteration_statement %type condition %type 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 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 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
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 Packard 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
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 McCabe 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 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: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/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 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 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 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 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 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 Packard wrote: On Fri, 17 Jun 2011 17:43:14 -0700, Dan McCabe 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
Re: [Mesa-dev] [PATCH 0/6] glsl: Add support for switch statements
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. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[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 (state->loop_o
[Mesa-dev] [PATCH 5/6] glsl: 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 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 selection_statement %type selection_rest_statement %type switch_statement -%type switch_body -%type case_label -%type case_label_list -%type case_statement -%type case_statement_list +%type switch_body +%type case_label_list +%type case_label +%type case_statement +%type case_statement_list %type iteration_statement %type condition %type 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: case_statement { - $$ = NULL; + ast_case_statement_list *cases= new(state) ast_
[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 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 struct_declarator_list %type selection_statement %type selection_rest_statement +%type switch_statement +%type switch_body +%type case_label +%type case_label_list +%type case_statement +%type case_statement_list %type iteration_statement %type condition %type 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 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 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 me
Re: [Mesa-dev] [PATCH 1/7] Create AST data structures for switch statement and case label
A valid criticism. It hadn't occured to me at the time, but you are right. The problem I was trying to sovle was gitting visibility to the test expression IR and the fall through state IR during case label processing. Another solution is to to add those IR pointers to struct _mesa_glsl_parse_state. I removed one such pointer (switch_or_loop_nesting) for other reasons, but adding other IR pointers to struct _mesa_glsl_parse_state doesn't seem to be an issue (at least not to previous implementors :). I'll look at it in more detail when I continue that work on Friday. Thanks. cheers, danm On Wed, Jun 15, 2011 at 11:59 AM, Kenneth Graunke wrote: > On 06/15/2011 09:33 AM, Dan McCabe wrote: > >> Data structures for switch statement and case label are created that >> parallel >> the structure of other AST data. >> --- >> src/glsl/ast.h | 27 +++ >> 1 files changed, 23 insertions(+), 4 deletions(-) >> > > Dan, thanks for sending this out! A few comments below... > > > diff --git a/src/glsl/ast.h b/src/glsl/ast.h >> index 878f48b..48d1795 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,21 @@ 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; >> + >> + ir_variable *test_var; >> > > Presumably this is "x" in "switch (x) { ... }"? That can be an arbitrary > scalar integer expression, not just a variable. > > > + ir_variable *fallthru_var; >> + >> +protected: >> + void test_to_hir(class ir_loop *, struct _mesa_glsl_parse_state *); >> }; >> > > I don't think we should be putting ir_variables (or any HIR code!) in the > AST structures. The AST should really just be an abstract syntax tree, > representing the text and structure of the program, but independent of any > IR code we might generate. > > I had imagined creating some kind of IR structure along the lines of: > > class ir_switch : public ir_instruction > { >ir_rvalue *switch_value; // switch (...) - arbitrary expression > >exec_list cases; // list of ir_case > }; > > class ir_case : public exec_node > { >ir_rvalue *test_value; // case X: ... NULL means "default:" >exec_list body; // statements inside this case >// possibly a bool has_break or falls_through, if helpful > }; > > This seems pretty straightforward to generate at AST->HIR time. > > Then there would be a lowering pass (see lower_*.cpp) based on an > ir_hierarchical_visitor that visits ir_switch, replacing it with the loop/if > structure. ir_variable *fallthru_var would fit nicely as a member of that > visitor. visit(ir_switch *) would create/emit it, and visit(ir_case *) > would reference it. > > Of course, there may be another solution; it's just an idea. > > --Kenneth > ___ 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 (state->loop_or
[Mesa-dev] [PATCH 7/7] Force break at end of generated loop
A loop is generated for the wrapper of the switch statement. We need to force the exit of that loop after all cases are processed. Therefore, generate an implicit break at the very end of the loop. --- src/glsl/ast_to_hir.cpp |8 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp index c409c2d..b4ba5a2 100644 --- a/src/glsl/ast_to_hir.cpp +++ b/src/glsl/ast_to_hir.cpp @@ -3364,9 +3364,17 @@ ir_rvalue * ast_switch_body::hir(exec_list *instructions, struct _mesa_glsl_parse_state *state) { + void *ctx = state; + if (stmts != NULL) stmts->hir(instructions, state); + // append break to force loop exit + ir_jump *const break_stmt = + new(ctx) ir_loop_jump(ir_loop_jump::jump_break); + + instructions->push_tail(break_stmt); + /* Switch bodies do not have r-values. */ return NULL; -- 1.7.4.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[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 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 selection_statement %type selection_rest_statement %type switch_statement -%type switch_body -%type case_label -%type case_label_list -%type case_statement -%type case_statement_list +%type switch_body +%type case_label_list +%type case_label +%type case_statement +%type case_statement_list %type iteration_statement %type condition %type 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: case_statement { - $$ = NULL; + ast_case_statement_list *cases= new(state) ast_
[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 1/7] 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 | 27 +++ 1 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/glsl/ast.h b/src/glsl/ast.h index 878f48b..48d1795 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,21 @@ 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; + + ir_variable *test_var; + ir_variable *fallthru_var; + +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/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 struct_declarator_list %type selection_statement %type selection_rest_statement +%type switch_statement +%type switch_body +%type case_label +%type case_label_list +%type case_statement +%type case_statement_list %type iteration_statement %type condition %type 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 0/7] 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 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 mes
[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