<div dir="ltr">Hi Tapani, Timothy,<div> </div><div>I also did some testing on my SNB laptop and didn't noticed any issues. CI also didn't show that issues.</div><div> </div><div>Regards,</div><div>Vadym <div> </div><div> </div></div> <div class="gmail_quote"><div dir="ltr">ср, 31 окт. 2018 г. в 9:13, Tapani Pälli <<a href="mailto:tapani.palli@intel.com" target="_blank">tapani.palli@intel.com</a>>: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> On 10/31/18 7:28 AM, Tapani Pälli wrote: > > > On 10/31/18 12:57 AM, Timothy Arceri wrote: >> On 30/10/18 10:14 pm, Tapani Pälli wrote: >>> Hi; >>> >>> On 10/30/18 1:28 AM, Timothy Arceri wrote: >>>> On 29/10/18 10:05 pm, Vadim Shovkoplias wrote: >>>>> Hi Timothy, >>>>> >>>>> Thanks for the review. Piglit patch is updated with the additional >>>>> out var: <a href="https://patchwork.freedesktop.org/patch/258899/" rel="noreferrer" target="_blank">https://patchwork.freedesktop.org/patch/258899/</a> >>>>> Original reporter confirmed that issue is finally fixed with the >>>>> current patch and closed it. >>>>> >>>>> Can I ask to push the patch please ? >>>> >>>> I've pushed both. Thanks for the patches! >>> >>> Unfortunately it seems this patch introduced regression in Intel CI, >>> I'm getting following failures after this commit (7d66eddbbde): >>> >>> piglit.spec.glsl-1_30.execution.tex-miplevel-selection >>> texturelodoffset 2dshadow.snbm64 >>> piglit.spec.glsl-1_30.execution.tex-miplevel-selection texturelod >>> 2dshadow.snbm64 >>> piglit.spec.glsl-1_30.execution.tex-miplevel-selection texturelod >>> cube.snbm64 >>> generated.gpu-hang-otc-gfxtest-snbgt2-02.snbm64 >> >> I don't have a Sandy Bridge to test with but its really strange that >> this would break only one generation of hardware. Are you sure it was >> this patch? > > Well .. not 100% sure but it looks like previous commit won't show these > fails, it is strange. I'll do some runs today and try to figure it out. Yeah, it passes now so it seems this was some kind of 'CI noise'. > >>> >>> >>>>> >>>>> Regards, >>>>> Vadym >>>>> >>>>> сб, 27 окт. 2018 г. в 1:21, Timothy Arceri <<a href="mailto:tarceri@itsqueeze.com" target="_blank">tarceri@itsqueeze.com</a> >>>>> <mailto:<a href="mailto:tarceri@itsqueeze.com" target="_blank">tarceri@itsqueeze.com</a>>>: >>>>> >>>>> On Wed, Oct 24, 2018, at 3:28 AM, Vadym Shovkoplias wrote: >>>>> > Since out variables are copied from shader objects instruction >>>>> > streams to linked shader instruction steam it should be cloned >>>>> > at first to keep source instruction steam unaltered. >>>>> > >>>>> > Fixes: 966a797e433 glsl/linker: Link all out vars from a shader >>>>> > objects on a single stage >>>>> > Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=105731" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=105731</a> >>>>> > Signed-off-by: Vadym Shovkoplias >>>>> <<a href="mailto:vadym.shovkoplias@globallogic.com" target="_blank">vadym.shovkoplias@globallogic.com</a> >>>>> <mailto:<a href="mailto:vadym.shovkoplias@globallogic.com" target="_blank">vadym.shovkoplias@globallogic.com</a>>> >>>>> >>>>> Reviewed-by: Timothy Arceri <<a href="mailto:tarceri@itsqueeze.com" target="_blank">tarceri@itsqueeze.com</a> >>>>> <mailto:<a href="mailto:tarceri@itsqueeze.com" target="_blank">tarceri@itsqueeze.com</a>>> >>>>> >>>>> Also please either update the existing piglit or add a new one to >>>>> better cover the use of the freed vars. From the bug report it >>>>> seems >>>>> adding another out var is enough to do it. Thanks. >>>>> >>>>> > --- >>>>> > src/compiler/glsl/linker.cpp | 3 ++- >>>>> > 1 file changed, 2 insertions(+), 1 deletion(-) >>>>> > >>>>> > diff --git a/src/compiler/glsl/linker.cpp >>>>> b/src/compiler/glsl/linker.cpp >>>>> > index 7db34ebf95..8b1b03322a 100644 >>>>> > --- a/src/compiler/glsl/linker.cpp >>>>> > +++ b/src/compiler/glsl/linker.cpp >>>>> > @@ -2269,10 +2269,11 @@ link_output_variables(struct >>>>> gl_linked_shader >>>>> > *linked_shader, >>>>> > if (ir->ir_type != ir_type_variable) >>>>> > continue; >>>>> > >>>>> > - ir_variable *const var = (ir_variable *) ir; >>>>> > + ir_variable *var = (ir_variable *) ir; >>>>> > >>>>> > if (var->data.mode == ir_var_shader_out && >>>>> > !symbols->get_variable(var->name)) { >>>>> > + var = var->clone(linked_shader, NULL); >>>>> > symbols->add_variable(var); >>>>> > linked_shader->ir->push_head(var); >>>>> > } >>>>> > -- >>>>> > 2.18.0 >>>>> > >>>>> </blockquote></div></div>