On 10/12/2017 08:04 AM, Jose Fonseca wrote:
The intent here was not so much to match the piglti MSVC build, but apps
build by MSVC in general.
After all, nothing ever prevented us from setting a huge stack size on
both MinGW and MSVC alike, as both toolchains allow to congifure the
stack size to whatever we want.
The key issue here is that OpenGL driver don't get to pick the apps they
are loaded, and real OpenGL applications will be likely built with MSVC
instead of Mingw, and therefore will likely only have the MSVC default
stack size. And we should err on the side of caution when testing.
Regardless of the compiler used, if we bump the stack size in piglit,
one is just increasing the chance that wasteful stack allocations go
undetected on piglit and blow up on real applications.
Therefore I suggest we continue to keep 1MB default, and try to fix Mesa
to be less stack hungry. If that's not pratical here,
The ir_expression::constant_expression_value() function's local vars are
pretty minimal. The two that stand out:
ir_constant *op[ARRAY_SIZE(this->operands)] = { NULL, };
ir_constant_data data;
are only 32 and 128 bytes, respectively (on a 32-bit build). I'm not
sure what else accounts for the approx 2KB of the activation record.
I don't see an obvious way to fix the problem. Even if we could reduce
per-call stack memory, someone could write an evil shader that adds a
thousand or more terms and we'd overflow the stack again.
then we should
try to bump the stack size of specific piglit tests (like those that
stress the compiler to the extreme, as Cmake allows to set these options
per executable), and only those tests. Or just mark the affected tests
as expected fail/skip.
The op-selection-bool-bvec4-bvec4.frag test is run with
shader_runner.exe. I think most of the large/complex shaders in Piglit
are run with shader_runner so I don't think it makes much difference if
only shader_runner or all piglit executables are build with a 2MB stack.
In any case, I've got a patch which only sets shader_runner's stack size
which I'll post next.
I guess another option is to change the
op-selection-bool-bvec4-bvec4.frag test to be simpler, but that's just
sweeping the root problem under the rug. Someone could always craft a
complex shader which blows out the stack.
If we feel that 1MB stack is too restrictive nowadays, then we should
invest time into moving GLSL compilation into a separate/internal
thread, which would enable us to pick whatever stack size we want when
creating that thread. Another alternative is to do a low-level
manipulation of stack registers, and force the compilation to happen in
manually allocated stack. Neither are easy-peasy though, and severaly
hamper debugability.
Yeah, those options don't sound too appealing or do-able in the short term.
-Brian
Jose
On 12/10/17 14:43, Brian Paul wrote:
The op-selection-bool-bvec4-bvec4.frag test has an expression which
adds 512 terms. This causes 512 levels of recursion in Mesa's
ir_expression::constant_expression_value() function. Since each
function activation record is about 2KB in size with MSVC, this
causes us to overflow the stack and crash.
The crash was only recently exposed with MSVC 2015 debug builds of Mesa.
The default MinGW stack size of 2MB is enough to avoid this issue.
Since we no longer build Piglit with MSVC, we don't have to match
MSVC's 1MB stack default.
---
CMakeLists.txt | 3 ---
1 file changed, 3 deletions(-)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 4259ec8..cdecca4 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -301,9 +301,6 @@ else ()
endif ()
if (MINGW)
- # Match MSVC default stack size
- set (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS}
-Wl,--stack,1048576")
-
# Avoid depending on MinGW runtime DLLs
check_cxx_compiler_flag (-static-libgcc HAVE_STATIC_LIBGCC_FLAG)
if (HAVE_STATIC_LIBGCC_FLAG)
_______________________________________________
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
