Re: [Mesa-dev] [PATCH 2/2] glsl: When linking, emit functions at the tail of the final linked program.
On 5 August 2011 18:29, Kenneth Graunke kenn...@whitecape.org wrote: On 08/03/2011 05:07 PM, Paul Berry wrote: When link_functions.cpp adds a new function to the final linked program, it needs to add it after any global variable declarations that the function refers to, otherwise the IR will be invalid (because variable declarations must occur before variable accesses). The easiest way to do that is to have the linker emit functions to the tail of the final linked program. The linker used to emit functions to the head of the final linked program, in an effort to keep callees sorted before their callers. However, this was not reliable: it didn't work for functions declared or defined in the same compilation unit as main, for diamond-shaped patterns in the call graph, or for some obscure cases involving overloaded functions. And no code currently relies on this sort order. No Piglit regressions with i965 Ironlake. Seems reasonable. Reviewed-by: Kenneth Graunke kenn...@whitecape.org That said, I still dislike all this nonsense. I've really wanted to separate these, so a shader contains three separate lists: 1. A list of global variable declarations (ir_variable *) 2. A list of function declarations (ir_function *) 3. Global initializer instructions (ir_instruction *) Then you wouldn't have any of this crap about emitting functions before variable declarations, functions in functions, etc, etc. Plus you wouldn't need to dynamically check whether things in your top-level list are variables/functions/etc. Quite some time ago I took a stab at this, but wasn't thinking about global initializers, so I made no provisions for them and ended up tossing the code. It'd be nice to resurrect the idea. If you'd like to try, feel free... :) That's a really good idea. I'm not sure I have the time to pour into this right now, but I'll keep it in mind if I have to visit this code again. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] glsl: When linking, emit functions at the tail of the final linked program.
On 4 August 2011 08:33, Eric Anholt e...@anholt.net wrote: On Wed, 3 Aug 2011 17:07:42 -0700, Paul Berry stereotype...@gmail.com wrote: When link_functions.cpp adds a new function to the final linked program, it needs to add it after any global variable declarations that the function refers to, otherwise the IR will be invalid (because variable declarations must occur before variable accesses). The easiest way to do that is to have the linker emit functions to the tail of the final linked program. The linker used to emit functions to the head of the final linked program, in an effort to keep callees sorted before their callers. However, this was not reliable: it didn't work for functions declared or defined in the same compilation unit as main, for diamond-shaped patterns in the call graph, or for some obscure cases involving overloaded functions. And no code currently relies on this sort order. Usually we'd swap the order of these around, so that if we have a testcase that would hit this, you don't get extra failures when a bisect lands on patch 1/2. That's a good point, Eric. I'll switch the order of the patches. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] glsl: When linking, emit functions at the tail of the final linked program.
On 08/03/2011 05:07 PM, Paul Berry wrote: When link_functions.cpp adds a new function to the final linked program, it needs to add it after any global variable declarations that the function refers to, otherwise the IR will be invalid (because variable declarations must occur before variable accesses). The easiest way to do that is to have the linker emit functions to the tail of the final linked program. The linker used to emit functions to the head of the final linked program, in an effort to keep callees sorted before their callers. However, this was not reliable: it didn't work for functions declared or defined in the same compilation unit as main, for diamond-shaped patterns in the call graph, or for some obscure cases involving overloaded functions. And no code currently relies on this sort order. No Piglit regressions with i965 Ironlake. Seems reasonable. Reviewed-by: Kenneth Graunke kenn...@whitecape.org That said, I still dislike all this nonsense. I've really wanted to separate these, so a shader contains three separate lists: 1. A list of global variable declarations (ir_variable *) 2. A list of function declarations (ir_function *) 3. Global initializer instructions (ir_instruction *) Then you wouldn't have any of this crap about emitting functions before variable declarations, functions in functions, etc, etc. Plus you wouldn't need to dynamically check whether things in your top-level list are variables/functions/etc. Quite some time ago I took a stab at this, but wasn't thinking about global initializers, so I made no provisions for them and ended up tossing the code. It'd be nice to resurrect the idea. If you'd like to try, feel free... :) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] glsl: When linking, emit functions at the tail of the final linked program.
On Wed, 3 Aug 2011 17:07:42 -0700, Paul Berry stereotype...@gmail.com wrote: When link_functions.cpp adds a new function to the final linked program, it needs to add it after any global variable declarations that the function refers to, otherwise the IR will be invalid (because variable declarations must occur before variable accesses). The easiest way to do that is to have the linker emit functions to the tail of the final linked program. The linker used to emit functions to the head of the final linked program, in an effort to keep callees sorted before their callers. However, this was not reliable: it didn't work for functions declared or defined in the same compilation unit as main, for diamond-shaped patterns in the call graph, or for some obscure cases involving overloaded functions. And no code currently relies on this sort order. Usually we'd swap the order of these around, so that if we have a testcase that would hit this, you don't get extra failures when a bisect lands on patch 1/2. pgp1UnG58uFg3.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] glsl: When linking, emit functions at the tail of the final linked program.
When link_functions.cpp adds a new function to the final linked
program, it needs to add it after any global variable declarations
that the function refers to, otherwise the IR will be invalid (because
variable declarations must occur before variable accesses). The
easiest way to do that is to have the linker emit functions to the
tail of the final linked program.
The linker used to emit functions to the head of the final linked
program, in an effort to keep callees sorted before their callers.
However, this was not reliable: it didn't work for functions declared
or defined in the same compilation unit as main, for diamond-shaped
patterns in the call graph, or for some obscure cases involving
overloaded functions. And no code currently relies on this sort
order.
No Piglit regressions with i965 Ironlake.
---
src/glsl/link_functions.cpp |6 --
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/glsl/link_functions.cpp b/src/glsl/link_functions.cpp
index 7ba760d..c0eb342 100644
--- a/src/glsl/link_functions.cpp
+++ b/src/glsl/link_functions.cpp
@@ -104,10 +104,12 @@ public:
if (f == NULL) {
f = new(linked) ir_function(name);
-/* Add the new function to the linked IR.
+/* Add the new function to the linked IR. Put it at the end
+ * so that it comes after any global variable declarations
+ * that it refers to.
*/
linked-symbols-add_function(f);
-linked-ir-push_head(f);
+linked-ir-push_tail(f);
}
ir_function_signature *linked_sig =
--
1.7.6
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
