| Summary: | Some extensions enabled even when not supported by the underlying driver | ||
|---|---|---|---|
| Product: | Mesa | Reporter: | Vinson Lee <vlee> |
| Component: | glsl-compiler | Assignee: | Paul Berry <stereotype441> |
| Status: | CLOSED FIXED | QA Contact: | |
| Severity: | critical | ||
| Priority: | medium | ||
| Version: | git | ||
| Hardware: | x86 (IA32) | ||
| OS: | Linux (All) | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
Shouldn't the compiler reject the shader? The driver doesn't seem to claim to support anything which would indicate the texture grad instructions are supported (namely glsl 1.30, EXT_gpu_shader4, ARB_shader_texture_lod) which would lead to TXD emit. Or the driver should just fail to link maybe... (In reply to comment #1) > Shouldn't the compiler reject the shader? The driver doesn't seem to claim to > support anything which would indicate the texture grad instructions are > supported (namely glsl 1.30, EXT_gpu_shader4, ARB_shader_texture_lod) which > would lead to TXD emit. Or the driver should just fail to link maybe... That's my thought too. It seems likely that something is going wrong in the extension checking in the compiler. I cannot get a shader using texture2DGrad or texture2DGradARB to compile on the i965 driver. Can you send the output of MESA_GLSL=dump? (In reply to comment #4) > I added piglit test case glsl-link-bug38015. Ah, that makes a bit more sense. It seems that we're not handling extension 'enable' for unsupported extensions correctly. The GLSL spec says, "Behave as specified by the extension extension_name. Warn on the #extension if the extension extension_name is not supported. Give an error on the #extension if all is specified." The first two statements seem to be in conflict. How can the driver "behave as specified by the extension extension_name" when the extension isn't supported? My thinking is that we should emit the warning, create the #define with the value 0, and not do anything else from the extension. I'm curious what other drivers do with 'enable' of extensions that they "know" but are not supported on a particular piece of hardware. I'm also going to change the component to the glsl-compiler since this doesn't really have anything to do with the i965 driver. (In reply to comment #5) > My thinking is that we should emit the warning, create the #define with the > value 0, and not do anything else from the extension. I take that back. If the extension is supported, the #define is set (whether it is enabled or not). All we need to do is emit the warning and NOT enable the extension. I believe the problem is that _mesa_glsl_process_extension() is checking whether the extension is supported *after* enabling/disabling/turning on warnings for it. It should be checking whether the extension is supported first, and then only taking action if it is. It looks like several other extensions have similar problems: - GL_ARB_draw_instanced - GL_ARB_explicit_attrib_location - GL_ARB_fragment_coord_conventions - GL_EXT_texture_array - GL_ARB_shader_stencil_export - GL_AMD_conservative_depth - GL_AMD_shader_stencil_export - GL_OES_texture_3D Assuming this explanation is correct, I'll try and fix all these in one patch. I would love to do this fix by replacing _mesa_glsl_process_extension() with some table-driven code, so that we don't make the same mistake when implementing future extensions, but since the extension enable flags are stored in bitfields that would probably require more invasive surgery than is really justified. Note: while investigating this bug, I believe I found two other, less severe bugs: 1. Whenever a shader mentions the GL_ARB_shader_texture_lod extension, we automatically turn on the GL_ARB_texture_rectangle extension, regardless of whether GL_ARB_shader_texture_lod is being enabled or disabled. 2. The shader directives "#extension all : warn" and "#extension all : disable" look like they have no effect. These two bugs are lower impact because their only effect is to suppress errors and warnings that would otherwise be generated. I'll file separate bugs for these two problems, and we can come back to them if time allows. Presumably the original problem (TXD assertion) is gone now that the i965 driver actually supports GL_ARB_shader_texture_lod...in particular, I deleted that assertion. :) But the #extension handling bug still stands. Perhaps this bug report should be renamed? (In reply to comment #9) > Presumably the original problem (TXD assertion) is gone now that the i965 > driver actually supports GL_ARB_shader_texture_lod...in particular, I deleted > that assertion. :) > > But the #extension handling bug still stands. Perhaps this bug report should > be renamed? Agreed. I've renamed the bug to reflect the extension handling problem that remains. I should have a patch submitted today or tomorrow to fix the problem. Fixed with commit 3097715d41da4b725b7ce9f9d5bbc0f684cbf0a6 mesa: 4a7667b96b7bd7cdffbe929182c15935b74facd2 (master) Verified fixed. |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.
mesa: 2292025c49f2165b59f578c926d320913b08b1b5 (master) chipset: GM45 libdrm-intel1: 2.4.25-2ubuntu2 xserver-xorg-video-intel: 2:2.15.0-3ubuntu1 kernel version: 2.6.39-3-generic Linux distribution: Ubuntu 11.10 i386 Open VMware Workstation or VMware Player. Power on a virtual machine. The i965 graphics driver will immediately hit an assertion. brw_fs_visitor.cpp:598: fs_inst* fs_visitor::emit_texture_gen4(ir_texture*, fs_reg, fs_reg, int): Assertion `!"TXD isn't supported on gen4 yet."' failed. (bt) [...] #12 0x00e3b416 in __kernel_vsyscall () #13 0x00417df1 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64 #14 0x0041b2ce in abort () at abort.c:92 #15 0x00410808 in __assert_fail (assertion=0x1eb1fd8 "!\"TXD isn't supported on gen4 yet.\"", file=0x1eb2034 "brw_fs_visitor.cpp", line=598, function=0x1eb2b80 "fs_inst* fs_visitor::emit_texture_gen4(ir_texture*, fs_reg, fs_reg, int)") at assert.c:81 #16 0x01ca043e in fs_visitor::emit_texture_gen4 (this=0x148e2c0, ir=0xbff7058, dst=..., coordinate=..., sampler=0) at brw_fs_visitor.cpp:598 #17 0x01ca0a5f in fs_visitor::visit (this=0x148e2c0, ir=0xbff7058) at brw_fs_visitor.cpp:941 #18 0x01e0d8c7 in ir_texture::accept (this=0xbff7058, v=0x148e2c0) at ir.h:1245 #19 0x01c97431 in fs_visitor::visit (this=0x148e2c0, ir=0xc077ef8) at brw_fs_visitor.cpp:510 #20 0x01e0d7b7 in ir_assignment::accept (this=0xc077ef8, v=0x148e2c0) at ir.h:703 #21 0x01c93427 in visit (this=0x148e2c0, ir=0xbf6e638) at brw_fs_visitor.cpp:1426 #22 fs_visitor::visit (this=0x148e2c0, ir=0xbf6e638) at brw_fs_visitor.cpp:1409 #23 0x01e0d737 in ir_function::accept (this=0xbf6e638, v=0x148e2c0) at ir.h:536 #24 0x01c8f240 in fs_visitor::run (this=0x148e2c0) at brw_fs.cpp:1534 #25 0x01c8f7fb in brw_wm_fs_emit (brw=0xbb4dbc8, c=0x6cec020, prog=0xc0cff00) at brw_fs.cpp:1624 #26 0x01c6c88c in do_wm_prog (brw=0xbb4dbc8, prog=0xc0cff00, fp=0xc04e790, key=0x149e7e0) at brw_wm.c:232 #27 0x01c8ff5d in brw_fs_precompile (ctx=0xbb4dbc8, prog=0xc0cff00) at brw_fs.cpp:1704 #28 0x01caa723 in brw_shader_precompile (ctx=0xbb4dbc8, prog=0xc0cff00) at brw_shader.cpp:67 #29 0x01df04fb in _mesa_glsl_link_shader (ctx=0xbb4dbc8, prog=0xc0cff00) at program/ir_to_mesa.cpp:3266 #30 0x01d1e066 in link_program (ctx=0xbb4dbc8, program=<value optimized out>) at main/shaderapi.c:877 [...]