Re: [Mesa-dev] [PATCH 1/2] glsl: validate IR after linking (debug builds only)

2011-08-05 Thread Kenneth Graunke
On 08/03/2011 05:07 PM, Paul Berry wrote:
 At least one of the invariants verified by IR validation concerns the
 relative ordering of toplevel constructs in the IR: references to
 global variables must come after the declarations of those global
 variables.
 
 Since linking affects the ordering of toplevel constructs in the IR,
 it's possible that a bug in the linker will cause invalid IR to be
 generated, even if all the pre-linked shaders are valid.  (In fact,
 such a bug currently exists.  I'll submit a follow-up patch which
 fixes it.)
 
 Bugs like this are easily masked by further optimization passes,
 particularly inlining.  So to make them easier to track down, this
 patch addes an IR validation step right after linking, and before
 final optimization occurs.  The validation only occurs on debug
 builds.

Really surprised to see that we weren't doing this.  It absolutely
should happen.  Though, I agree with Eric...reorder them so this becomes
patch #2.

Reviewed-by: Kenneth Graunke kenn...@whitecape.org
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 1/2] glsl: validate IR after linking (debug builds only)

2011-08-03 Thread Paul Berry
At least one of the invariants verified by IR validation concerns the
relative ordering of toplevel constructs in the IR: references to
global variables must come after the declarations of those global
variables.

Since linking affects the ordering of toplevel constructs in the IR,
it's possible that a bug in the linker will cause invalid IR to be
generated, even if all the pre-linked shaders are valid.  (In fact,
such a bug currently exists.  I'll submit a follow-up patch which
fixes it.)

Bugs like this are easily masked by further optimization passes,
particularly inlining.  So to make them easier to track down, this
patch addes an IR validation step right after linking, and before
final optimization occurs.  The validation only occurs on debug
builds.
---
 src/glsl/linker.cpp |8 
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index fe570b6..df7de18 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -910,6 +910,14 @@ link_intrastage_shaders(void *mem_ctx,
 
free(linking_shaders);
 
+#ifdef DEBUG
+   /* At this point linked should contain all of the linked IR, so
+* validate it to make sure nothing went wrong.
+*/
+   if (linked)
+  validate_ir_tree(linked-ir);
+#endif
+
/* Make a pass over all variable declarations to ensure that arrays with
 * unspecified sizes have a size specified.  The size is inferred from the
 * max_array_access field.
-- 
1.7.6

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev