| Summary: | [G965 ILK G45] Regression: 24 piglit regressions in glsl-1.10 | ||
|---|---|---|---|
| Product: | Mesa | Reporter: | Mark Janes <mark.a.janes> |
| Component: | Drivers/DRI/i965 | Assignee: | Alejandro Piñeiro (freenode IRC: apinheiro) <apinheiro> |
| Status: | CLOSED FIXED | QA Contact: | Intel 3D Bugs Mailing List <intel-3d-bugs> |
| Severity: | blocker | ||
| Priority: | high | CC: | mark.a.janes, mattst88 |
| Version: | git | ||
| Hardware: | x86-64 (AMD64) | ||
| OS: | Linux (All) | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
|
Description
Mark Janes
2015-10-22 21:44:26 UTC
(In reply to Mark Janes from comment #0) > The following commit series produced many regressions on older intel > hardware: Unfourtunately, I don't have access to this hw at home. Will check if I can borrow this old hw from one of my colleagues. Sorry for the hassle. > 8ac3b525c77cb5aae9e61bd984b78f6cbbffdc1c - > 8cf84a7e470dbd3b46ce4081459d2ecfab22c2d5 > > Author: Alejandro Piñeiro <apinheiro@igalia.com> > AuthorDate: Fri Oct 9 18:39:42 2015 +0200 > Commit: Alejandro Piñeiro <apinheiro@igalia.com> > CommitDate: Thu Oct 22 21:58:03 2015 +0200 > > i965/vec4: print predicate control at brw_vec4 dump_instruction > > v2: externalize pred_ctrl_align16 from brw_disasm.c instead of adding > a copy on brw_vec4.c, as suggested by Matt Turner > > Reviewed-by: Matt Turner <mattst88@gmail.com> > > > The failing tests: > glsl-1_20.execution.clipping.vs-clip-vertex-homogeneity > glsl-1_10.execution.interpolation.interpolation-none-gl_frontsecondarycolor- > smooth-vertex > glsl-1_10.execution.interpolation.interpolation-none-gl_backcolor-smooth- > vertex > glsl-1_10.execution.clipping.clip-plane-transformation pos_clipvert > glsl-1_10.execution.interpolation.interpolation-none-other-flat-vertex > glsl-1_10.execution.clipping.clip-plane-transformation clipvert_pos > glsl-1_10.execution.interpolation.interpolation-none-gl_backsecondarycolor- > smooth-vertex > glsl-1_20.execution.clipping.vs-clip-vertex-const-accept > glsl-1_10.execution.clipping.clip-plane-transformation fixed > glsl-1_20.execution.clipping.vs-clip-vertex-equal-to-position > glsl-1_20.execution.clipping.vs-clip-vertex-enables > glsl-1_20.execution.clipping.fixed-clip-enables !opengl 1_1.user-clip > glsl-1_10.execution.interpolation.interpolation-none-other-smooth-vertex > arb_vertex_program.clip-plane-transformation arb !opengl > 1_0.gl-1.0-fpexceptions > glsl-1_10.execution.interpolation.interpolation-none-gl_frontsecondarycolor- > flat-vertex > glsl-1_10.execution.interpolation.interpolation-none-gl_backcolor-flat-vertex > glsl-1_10.execution.interpolation.interpolation-none-gl_frontcolor-flat- > vertex > glsl-1_20.execution.clipping.vs-clip-vertex-different-from-position > glsl-1_20.execution.clipping.vs-clip-vertex-const-reject > glsl-1_10.execution.interpolation.interpolation-none-gl_frontcolor-smooth- > vertex > glsl-1_10.execution.interpolation.interpolation-none-gl_backsecondarycolor- > flat-vertex (In reply to Alejandro Piñeiro (freenode IRC: apinheiro) from comment #1) > (In reply to Mark Janes from comment #0) > > The following commit series produced many regressions on older intel > > hardware: > > Unfourtunately, I don't have access to this hw at home. Will check if I can > borrow this old hw from one of my colleagues. I was able to get an ironlake device, and was able to confirm the regressions. Will start to work to see what happened. FWIW, the commit causing this problem is the following one: commit a59359ecd22154cc2b3f88bb8c599f21af8a3934 Author: Alejandro Piñeiro <apinheiro@igalia.com> Date: Wed Oct 14 20:26:43 2015 +0200 i965/vec4: track and use independently each flag channel vec4_live_variables tracks now each flag channel independently, so vec4_dead_code_eliminate can update the writemask of null registers, based on which component are alive at the moment. This would allow vec4_cmod_propagation to optimize out several movs involving null registers. v2: added support to track each flag channel independently at vec4 live_variables, as v1 assumed that it was already doing it, as pointed by Francisco Jerez v3: general cleaningn after Matt Turner's review Reviewed-by: Matt Turner <mattst88@gmail.com> That was also the last one to be tested. Removing this commit remove the regressions. Will investigate a little this one. Ok, I found the problem. Some comparisons are being wrongly removed with unpack_flags.
When I added this function:
bool reads_flag(unsigned c)
{
if (!reads_flag())
return false;
switch (predicate) {
case BRW_PREDICATE_NONE:
return false;
case BRW_PREDICATE_ALIGN16_REPLICATE_X:
return c == 0;
case BRW_PREDICATE_ALIGN16_REPLICATE_Y:
return c == 1;
case BRW_PREDICATE_ALIGN16_REPLICATE_Z:
return c == 2;
case BRW_PREDICATE_ALIGN16_REPLICATE_W:
return c == 3;
default:
return true;
}
}
I was only checking the predicate, but as the one without parameter shows:
bool reads_flag()
{
return predicate || opcode == VS_OPCODE_UNPACK_FLAGS_SIMD4X2;
}
We also need to check the opcode.
Adding that checks fixes the regressions I tested manually. I will do a full test and send a patch to the list soon.
(In reply to Mark Janes from comment #0) > The following commit series produced many regressions on older intel > hardware: > > 8ac3b525c77cb5aae9e61bd984b78f6cbbffdc1c - > 8cf84a7e470dbd3b46ce4081459d2ecfab22c2d5 > > Author: Alejandro Piñeiro <apinheiro@igalia.com> > AuthorDate: Fri Oct 9 18:39:42 2015 +0200 > Commit: Alejandro Piñeiro <apinheiro@igalia.com> > CommitDate: Thu Oct 22 21:58:03 2015 +0200 > > i965/vec4: print predicate control at brw_vec4 dump_instruction > > v2: externalize pred_ctrl_align16 from brw_disasm.c instead of adding > a copy on brw_vec4.c, as suggested by Matt Turner > > Reviewed-by: Matt Turner <mattst88@gmail.com> Patch that solved the bug just sent to the list: http://lists.freedesktop.org/archives/mesa-dev/2015-October/098207.html Having said so: > The failing tests: The title mentions 24 regressions, but only 21 are listed below. Additionally, as I mention on the email to freedesktop, on ILK I only detected 17. Could you confirm how many regressions my series introduced? > glsl-1_20.execution.clipping.vs-clip-vertex-homogeneity > glsl-1_10.execution.interpolation.interpolation-none-gl_frontsecondarycolor- > smooth-vertex > glsl-1_10.execution.interpolation.interpolation-none-gl_backcolor-smooth- > vertex > glsl-1_10.execution.clipping.clip-plane-transformation pos_clipvert > glsl-1_10.execution.interpolation.interpolation-none-other-flat-vertex > glsl-1_10.execution.clipping.clip-plane-transformation clipvert_pos > glsl-1_10.execution.interpolation.interpolation-none-gl_backsecondarycolor- > smooth-vertex > glsl-1_20.execution.clipping.vs-clip-vertex-const-accept > glsl-1_10.execution.clipping.clip-plane-transformation fixed > glsl-1_20.execution.clipping.vs-clip-vertex-equal-to-position > glsl-1_20.execution.clipping.vs-clip-vertex-enables > glsl-1_20.execution.clipping.fixed-clip-enables !opengl 1_1.user-clip > glsl-1_10.execution.interpolation.interpolation-none-other-smooth-vertex > arb_vertex_program.clip-plane-transformation arb !opengl > 1_0.gl-1.0-fpexceptions > glsl-1_10.execution.interpolation.interpolation-none-gl_frontsecondarycolor- > flat-vertex > glsl-1_10.execution.interpolation.interpolation-none-gl_backcolor-flat-vertex > glsl-1_10.execution.interpolation.interpolation-none-gl_frontcolor-flat- > vertex > glsl-1_20.execution.clipping.vs-clip-vertex-different-from-position > glsl-1_20.execution.clipping.vs-clip-vertex-const-reject > glsl-1_10.execution.interpolation.interpolation-none-gl_frontcolor-smooth- > vertex > glsl-1_10.execution.interpolation.interpolation-none-gl_backsecondarycolor- > flat-vertex (In reply to Alejandro Piñeiro (freenode IRC: apinheiro) from comment #5) > (In reply to Mark Janes from comment #0) > > The following commit series produced many regressions on older intel > > hardware: > > > > 8ac3b525c77cb5aae9e61bd984b78f6cbbffdc1c - > > 8cf84a7e470dbd3b46ce4081459d2ecfab22c2d5 > > > > Author: Alejandro Piñeiro <apinheiro@igalia.com> > > AuthorDate: Fri Oct 9 18:39:42 2015 +0200 > > Commit: Alejandro Piñeiro <apinheiro@igalia.com> > > CommitDate: Thu Oct 22 21:58:03 2015 +0200 > > > > i965/vec4: print predicate control at brw_vec4 dump_instruction > > > > v2: externalize pred_ctrl_align16 from brw_disasm.c instead of adding > > a copy on brw_vec4.c, as suggested by Matt Turner > > > > Reviewed-by: Matt Turner <mattst88@gmail.com> > > Patch that solved the bug just sent to the list: > http://lists.freedesktop.org/archives/mesa-dev/2015-October/098207.html Kenneth Graunke just reviewed the patch. I have just pushed it. I hope this solves all the regressions in all hw. all tests 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.