Commit bbf8291bf869e219bd0e71063bf26a060682a000 regresses the following test on BDW, BSW, BYT, HSW, IVB, SKL: piglit.spec.arb_gpu_shader5.execution.sampler_array_indexing.gs-struct-nonconst-sampler-nonconst Before NIR was enabled for vertex shaders, the test failed. With NIR, the test crashes. shader_runner /tmp/build_root/m32/lib/piglit/tests/spec/arb_gpu_shader5/execution/sampler_array_indexing/gs-struct-nonconst-sampler-nonconst.shader_test -auto stderr: shader_runner: /var/lib/jenkins/jobs/Leeroy/workspace/repos/mesa/src/glsl/nir/nir_validate.c:427: validate_tex_instr: Assertion `!src_type_seen[instr->src[i].src_type]' failed.
I took a look at this one. It actually does an arra-of-arrays operation. (I'm not sure why the shader compiled before.) I *think* that Timothy's NIR patches for AoA should fix it. I'm reassigning this to him. Without AoA, it should safely fail in release mode (not crash) so I don't think this is a big deal blocker.
(In reply to Jason Ekstrand from comment #1) > I took a look at this one. It actually does an arra-of-arrays operation. Not exactly. It's an array of structures that contain an array. That has been valid since GLSL 1.10 (i.e., forever). This is one of the reasons that it was so absurd that arrays-of-arrays weren't previously supported... you could still get the functionality with really ugly syntax.
This isn't an AoA its a struct array with a member array(In reply to Ian Romanick from comment #2) > (In reply to Jason Ekstrand from comment #1) > > I took a look at this one. It actually does an arra-of-arrays operation. > > Not exactly. It's an array of structures that contain an array. That has > been valid since GLSL 1.10 (i.e., forever). This is one of the reasons that > it was so absurd that arrays-of-arrays weren't previously supported... you > could still get the functionality with really ugly syntax. Yep. I've started working on a fix for this I hope to have it out shortly.
Since NIR for vec4 is available on 11.0 (but A-of-A is not?), I hope we'll be able to cherry pick the patches back...
(In reply to Ian Romanick from comment #4) > Since NIR for vec4 is available on 11.0 (but A-of-A is not?), I hope we'll > be able to cherry pick the patches back... Given that this is currently broken for SIMD8 (It turns out we have exactly 1 piglit test), we probably want to back-port the fix anyway.
(In reply to Ian Romanick from comment #4) > Since NIR for vec4 is available on 11.0 (but A-of-A is not?), I hope we'll > be able to cherry pick the patches back... Yes I'm fixing the non AoA case first, indirect indexing of structs that contain samplers was already broken so I hadn't added AoA support yet. I'll also need a few extra piglit tests for various crazyness such as multiple samplers in the nested structs.
In the end the solution I've come up with may be a little invasive for stable as I took this as an opportunity to remove more uses of the UniformHash table that I believe we shouldn't have to carry around anymore but I'm pretty happy with the fix, please review. Patches: http://lists.freedesktop.org/archives/mesa-dev/2015-September/093247.html
(In reply to Timothy Arceri from comment #7) > In the end the solution I've come up with may be a little invasive for > stable as I took this as an opportunity to remove more uses of the > UniformHash table that I believe we shouldn't have to carry around anymore > but I'm pretty happy with the fix, please review. I'm planning to try and review that in the next couple of days. As far as stable goes, I'm not sure what I think. I agree that the changes are big enough that we may not want to back-port them. We could leave it as-is given that the "regression" only happens in release builds. We could also back-port just the NIR patch to get rid of the crash as long as it, by itself, doesn't break anything further; however I feel a bit dirty about that.
These tests should now pass in master starting with commit ef8eebc6ad As this is only a regression in debug builds, and there are no bug reports of this issue in real apps I'm not planning to backport this to stable given how invasive the series of patches are.
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.