Re: [Mesa-dev] [PATCH] intel: decoder: handle 0 sized structs
Reviewed-by: Jason Ekstrand On Sat, Aug 25, 2018 at 12:23 PM Lionel Landwerlin < lionel.g.landwer...@intel.com> wrote: > Gen7.5 has a BLEND_STATE of size 0 which includes a variable length > group. We did not deal with that very well, leading to an endless > loop. > > Signed-off-by: Lionel Landwerlin > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107544 > --- > src/intel/common/gen_decoder.c | 12 > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/src/intel/common/gen_decoder.c > b/src/intel/common/gen_decoder.c > index 9e46f271633..ec22b545492 100644 > --- a/src/intel/common/gen_decoder.c > +++ b/src/intel/common/gen_decoder.c > @@ -997,7 +997,7 @@ gen_field_iterator_init(struct gen_field_iterator > *iter, > iter->p_bit = p_bit; > > int length = gen_group_get_length(iter->group, iter->p); > - iter->p_end = length > 0 ? [length] : NULL; > + iter->p_end = length >= 0 ? [length] : NULL; > iter->print_colors = print_colors; > } > > @@ -1012,10 +1012,14 @@ gen_field_iterator_next(struct gen_field_iterator > *iter) > iter_start_field(iter, iter->group->next->fields); > >bool result = iter_decode_field(iter); > - if (iter->p_end) > - assert(result); > + if (!result && iter->p_end) { > + /* We're dealing with a non empty struct of length=0 > (BLEND_STATE on > + * Gen 7.5) > + */ > + assert(iter->group->dw_length == 0); > + } > > - return true; > + return result; > } > > if (!iter_advance_field(iter)) > -- > 2.18.0 > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 77449] Tracker bug for all bugs related to Steam titles
https://bugs.freedesktop.org/show_bug.cgi?id=77449 Bug 77449 depends on bug 104809, which changed state. Bug 104809 Summary: anv: DOOM 2016 and Wolfenstein II:The New Colossus crash due to not having depthBoundsTest https://bugs.freedesktop.org/show_bug.cgi?id=104809 What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] anv: Fill holes in the VF VUE to zero
On Sat, Aug 25, 2018 at 5:56 PM Lionel Landwerlin < lionel.g.landwer...@intel.com> wrote: > I don't know how you find this stuff... > Hours and hours of painfully reading through aub dumps looking for stuff that's out of place and many experiments. This hang has probably taken me near enough to a week of debugging time all told. > Reviewed-by: Lionel Landwerlin > Thanks! --Jason > On 25/08/2018 23:17, Jason Ekstrand wrote: > > Cc: mesa-sta...@lists.freedesktop.org > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104809 > > --- > > src/intel/vulkan/genX_pipeline.c | 29 - > > 1 file changed, 28 insertions(+), 1 deletion(-) > > > > diff --git a/src/intel/vulkan/genX_pipeline.c > b/src/intel/vulkan/genX_pipeline.c > > index 022f324606e..b531205508c 100644 > > --- a/src/intel/vulkan/genX_pipeline.c > > +++ b/src/intel/vulkan/genX_pipeline.c > > @@ -115,7 +115,34 @@ emit_vertex_input(struct anv_pipeline *pipeline, > > GENX(3DSTATE_VERTEX_ELEMENTS)); > > if (!p) > > return; > > - memset(p + 1, 0, (num_dwords - 1) * 4); > > + > > + for (uint32_t i = 0; i < total_elems; i++) { > > + /* The SKL docs for VERTEX_ELEMENT_STATE say: > > + * > > + *"All elements must be valid from Element[0] to the last > valid > > + *element. (I.e. if Element[2] is valid then Element[1] and > > + *Element[0] must also be valid)." > > + * > > + * The SKL docs for 3D_Vertex_Component_Control say: > > + * > > + *"Don't store this component. (Not valid for Component 0, > but can > > + *be used for Component 1-3)." > > + * > > + * So we can't just leave a vertex element blank and hope for the > best. > > + * We have to tell the VF hardware to put something in it; so we > just > > + * store a bunch of zero. > > + * > > + * TODO: Compact vertex elements so we never end up with holes. > > + */ > > + struct GENX(VERTEX_ELEMENT_STATE) element = { > > + .Valid = true, > > + .Component0Control = VFCOMP_STORE_0, > > + .Component1Control = VFCOMP_STORE_0, > > + .Component2Control = VFCOMP_STORE_0, > > + .Component3Control = VFCOMP_STORE_0, > > + }; > > + GENX(VERTEX_ELEMENT_STATE_pack)(NULL, [1 + i * 2], ); > > + } > > > > for (uint32_t i = 0; i < info->vertexAttributeDescriptionCount; > i++) { > > const VkVertexInputAttributeDescription *desc = > > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] anv: Fill holes in the VF VUE to zero
I don't know how you find this stuff... Reviewed-by: Lionel Landwerlin On 25/08/2018 23:17, Jason Ekstrand wrote: Cc: mesa-sta...@lists.freedesktop.org Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104809 --- src/intel/vulkan/genX_pipeline.c | 29 - 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/src/intel/vulkan/genX_pipeline.c b/src/intel/vulkan/genX_pipeline.c index 022f324606e..b531205508c 100644 --- a/src/intel/vulkan/genX_pipeline.c +++ b/src/intel/vulkan/genX_pipeline.c @@ -115,7 +115,34 @@ emit_vertex_input(struct anv_pipeline *pipeline, GENX(3DSTATE_VERTEX_ELEMENTS)); if (!p) return; - memset(p + 1, 0, (num_dwords - 1) * 4); + + for (uint32_t i = 0; i < total_elems; i++) { + /* The SKL docs for VERTEX_ELEMENT_STATE say: + * + *"All elements must be valid from Element[0] to the last valid + *element. (I.e. if Element[2] is valid then Element[1] and + *Element[0] must also be valid)." + * + * The SKL docs for 3D_Vertex_Component_Control say: + * + *"Don't store this component. (Not valid for Component 0, but can + *be used for Component 1-3)." + * + * So we can't just leave a vertex element blank and hope for the best. + * We have to tell the VF hardware to put something in it; so we just + * store a bunch of zero. + * + * TODO: Compact vertex elements so we never end up with holes. + */ + struct GENX(VERTEX_ELEMENT_STATE) element = { + .Valid = true, + .Component0Control = VFCOMP_STORE_0, + .Component1Control = VFCOMP_STORE_0, + .Component2Control = VFCOMP_STORE_0, + .Component3Control = VFCOMP_STORE_0, + }; + GENX(VERTEX_ELEMENT_STATE_pack)(NULL, [1 + i * 2], ); + } for (uint32_t i = 0; i < info->vertexAttributeDescriptionCount; i++) { const VkVertexInputAttributeDescription *desc = ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel/tools: Add 0x in front of a couple of hex values
Reviewed-by: Lionel Landwerlin On 25/08/2018 23:34, Jason Ekstrand wrote: --- src/intel/common/gen_batch_decoder.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/intel/common/gen_batch_decoder.c b/src/intel/common/gen_batch_decoder.c index 6884a999401..1a5c8c37968 100644 --- a/src/intel/common/gen_batch_decoder.c +++ b/src/intel/common/gen_batch_decoder.c @@ -248,11 +248,11 @@ dump_binding_table(struct gen_batch_decode_ctx *ctx, uint32_t offset, int count) if (pointers[i] % 32 != 0 || addr < bo.addr || addr + size >= bo.addr + bo.size) { - fprintf(ctx->fp, "pointer %u: %08x \n", i, pointers[i]); + fprintf(ctx->fp, "pointer %u: 0x%08x \n", i, pointers[i]); continue; } - fprintf(ctx->fp, "pointer %u: %08x\n", i, pointers[i]); + fprintf(ctx->fp, "pointer %u: 0x%08x\n", i, pointers[i]); ctx_print_group(ctx, strct, addr, bo.map + (addr - bo.addr)); } } ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 105371] r600_shader_from_tgsi - GPR limit exceeded - shader requires 360 registers
https://bugs.freedesktop.org/show_bug.cgi?id=105371 --- Comment #16 from mirh --- (In reply to mirh from comment #5) > (In reply to mirh from comment #1) > > Can confirm it fixes shader 2 and 5 of GraphicsFuzz demo > > http://www.graphicsfuzz.com/benchmark/android-v1.html > > > > Should I wait for this (or, I dunno, some day sw fp64) to land before > > reporting of the others "gcm_sched_late_pass: unscheduled ops" errors? > > Well, colour me shocked, but after building mesa-git with the last patch > series all the tests now pass. > > Which is quite remarkable considering not even latest GCN closed drivers are > compliant. This with firefox though. Just noticed chromium 68 is still getting those errors for SETGT_DX10 and MULADD_IEEE, and indeed fails six tests. Nosb fixes it. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] anv: Fill holes in the VF VUE to zero
Added to the commit message: This fixes a GPU hang in DOOM 2016 running under wine. On Sat, Aug 25, 2018 at 5:17 PM Jason Ekstrand wrote: > Cc: mesa-sta...@lists.freedesktop.org > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104809 > --- > src/intel/vulkan/genX_pipeline.c | 29 - > 1 file changed, 28 insertions(+), 1 deletion(-) > > diff --git a/src/intel/vulkan/genX_pipeline.c > b/src/intel/vulkan/genX_pipeline.c > index 022f324606e..b531205508c 100644 > --- a/src/intel/vulkan/genX_pipeline.c > +++ b/src/intel/vulkan/genX_pipeline.c > @@ -115,7 +115,34 @@ emit_vertex_input(struct anv_pipeline *pipeline, > GENX(3DSTATE_VERTEX_ELEMENTS)); > if (!p) >return; > - memset(p + 1, 0, (num_dwords - 1) * 4); > + > + for (uint32_t i = 0; i < total_elems; i++) { > + /* The SKL docs for VERTEX_ELEMENT_STATE say: > + * > + *"All elements must be valid from Element[0] to the last valid > + *element. (I.e. if Element[2] is valid then Element[1] and > + *Element[0] must also be valid)." > + * > + * The SKL docs for 3D_Vertex_Component_Control say: > + * > + *"Don't store this component. (Not valid for Component 0, but > can > + *be used for Component 1-3)." > + * > + * So we can't just leave a vertex element blank and hope for the > best. > + * We have to tell the VF hardware to put something in it; so we > just > + * store a bunch of zero. > + * > + * TODO: Compact vertex elements so we never end up with holes. > + */ > + struct GENX(VERTEX_ELEMENT_STATE) element = { > + .Valid = true, > + .Component0Control = VFCOMP_STORE_0, > + .Component1Control = VFCOMP_STORE_0, > + .Component2Control = VFCOMP_STORE_0, > + .Component3Control = VFCOMP_STORE_0, > + }; > + GENX(VERTEX_ELEMENT_STATE_pack)(NULL, [1 + i * 2], ); > + } > > for (uint32_t i = 0; i < info->vertexAttributeDescriptionCount; i++) { >const VkVertexInputAttributeDescription *desc = > -- > 2.17.1 > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel/tools: Add 0x in front of a couple of hex values
--- src/intel/common/gen_batch_decoder.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/intel/common/gen_batch_decoder.c b/src/intel/common/gen_batch_decoder.c index 6884a999401..1a5c8c37968 100644 --- a/src/intel/common/gen_batch_decoder.c +++ b/src/intel/common/gen_batch_decoder.c @@ -248,11 +248,11 @@ dump_binding_table(struct gen_batch_decode_ctx *ctx, uint32_t offset, int count) if (pointers[i] % 32 != 0 || addr < bo.addr || addr + size >= bo.addr + bo.size) { - fprintf(ctx->fp, "pointer %u: %08x \n", i, pointers[i]); + fprintf(ctx->fp, "pointer %u: 0x%08x \n", i, pointers[i]); continue; } - fprintf(ctx->fp, "pointer %u: %08x\n", i, pointers[i]); + fprintf(ctx->fp, "pointer %u: 0x%08x\n", i, pointers[i]); ctx_print_group(ctx, strct, addr, bo.map + (addr - bo.addr)); } } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/3] nir: Create sampler variables in prog_to_nir.
On Saturday, August 25, 2018 6:05:57 AM PDT Jason Ekstrand wrote: > On Fri, Aug 24, 2018 at 8:24 PM Kenneth Graunke > wrote: > > > This is needed for nir_gather_info to actually count the textures, > > since it operates solely on variables. > > --- > > src/mesa/program/prog_to_nir.c | 15 +-- > > 1 file changed, 13 insertions(+), 2 deletions(-) > > > > diff --git a/src/mesa/program/prog_to_nir.c > > b/src/mesa/program/prog_to_nir.c > > index 14e57b6c6a1..1f0607542e8 100644 > > --- a/src/mesa/program/prog_to_nir.c > > +++ b/src/mesa/program/prog_to_nir.c > > @@ -52,6 +52,7 @@ struct ptn_compile { > > nir_variable *parameters; > > nir_variable *input_vars[VARYING_SLOT_MAX]; > > nir_variable *output_vars[VARYING_SLOT_MAX]; > > + nir_variable *sampler_vars[32]; /* matches number of bits in > > TexSrcUnit */ > > nir_register **output_regs; > > nir_register **temp_regs; > > > > @@ -484,9 +485,10 @@ ptn_kil(nir_builder *b, nir_ssa_def **src) > > } > > > > static void > > -ptn_tex(nir_builder *b, nir_alu_dest dest, nir_ssa_def **src, > > +ptn_tex(struct ptn_compile *c, nir_alu_dest dest, nir_ssa_def **src, > > struct prog_instruction *prog_inst) > > { > > + nir_builder *b = >build; > > nir_tex_instr *instr; > > nir_texop op; > > unsigned num_srcs; > > @@ -568,6 +570,15 @@ ptn_tex(nir_builder *b, nir_alu_dest dest, > > nir_ssa_def **src, > >unreachable("can't reach"); > > } > > > > + if (!c->sampler_vars[prog_inst->TexSrcUnit]) { > > + const struct glsl_type *type = > > + glsl_sampler_type(instr->sampler_dim, false, false, > > GLSL_TYPE_FLOAT); > > + nir_variable *var = > > + nir_variable_create(b->shader, nir_var_uniform, type, "sampler"); > > + var->data.binding = prog_inst->TexSrcUnit; > > + c->sampler_vars[prog_inst->TexSrcUnit] = var; > > > > Can samplers be indirected? If so, we probably want an array of samplers > instead 32 distinct samplers. That would be a frill. This is ARB_fragment_program. You get an integer constant for your texture unit number. :) --Ken signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] anv: Fill holes in the VF VUE to zero
Cc: mesa-sta...@lists.freedesktop.org Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104809 --- src/intel/vulkan/genX_pipeline.c | 29 - 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/src/intel/vulkan/genX_pipeline.c b/src/intel/vulkan/genX_pipeline.c index 022f324606e..b531205508c 100644 --- a/src/intel/vulkan/genX_pipeline.c +++ b/src/intel/vulkan/genX_pipeline.c @@ -115,7 +115,34 @@ emit_vertex_input(struct anv_pipeline *pipeline, GENX(3DSTATE_VERTEX_ELEMENTS)); if (!p) return; - memset(p + 1, 0, (num_dwords - 1) * 4); + + for (uint32_t i = 0; i < total_elems; i++) { + /* The SKL docs for VERTEX_ELEMENT_STATE say: + * + *"All elements must be valid from Element[0] to the last valid + *element. (I.e. if Element[2] is valid then Element[1] and + *Element[0] must also be valid)." + * + * The SKL docs for 3D_Vertex_Component_Control say: + * + *"Don't store this component. (Not valid for Component 0, but can + *be used for Component 1-3)." + * + * So we can't just leave a vertex element blank and hope for the best. + * We have to tell the VF hardware to put something in it; so we just + * store a bunch of zero. + * + * TODO: Compact vertex elements so we never end up with holes. + */ + struct GENX(VERTEX_ELEMENT_STATE) element = { + .Valid = true, + .Component0Control = VFCOMP_STORE_0, + .Component1Control = VFCOMP_STORE_0, + .Component2Control = VFCOMP_STORE_0, + .Component3Control = VFCOMP_STORE_0, + }; + GENX(VERTEX_ELEMENT_STATE_pack)(NULL, [1 + i * 2], ); + } for (uint32_t i = 0; i < info->vertexAttributeDescriptionCount; i++) { const VkVertexInputAttributeDescription *desc = -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel: decoder: handle 0 sized structs
Gen7.5 has a BLEND_STATE of size 0 which includes a variable length group. We did not deal with that very well, leading to an endless loop. Signed-off-by: Lionel Landwerlin Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107544 --- src/intel/common/gen_decoder.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c index 9e46f271633..ec22b545492 100644 --- a/src/intel/common/gen_decoder.c +++ b/src/intel/common/gen_decoder.c @@ -997,7 +997,7 @@ gen_field_iterator_init(struct gen_field_iterator *iter, iter->p_bit = p_bit; int length = gen_group_get_length(iter->group, iter->p); - iter->p_end = length > 0 ? [length] : NULL; + iter->p_end = length >= 0 ? [length] : NULL; iter->print_colors = print_colors; } @@ -1012,10 +1012,14 @@ gen_field_iterator_next(struct gen_field_iterator *iter) iter_start_field(iter, iter->group->next->fields); bool result = iter_decode_field(iter); - if (iter->p_end) - assert(result); + if (!result && iter->p_end) { + /* We're dealing with a non empty struct of length=0 (BLEND_STATE on + * Gen 7.5) + */ + assert(iter->group->dw_length == 0); + } - return true; + return result; } if (!iter_advance_field(iter)) -- 2.18.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel: tools: Fix aubinator_error's fprintf call (format-security)
Hey Lionel, Lionel Landwerlin wrote on 8/25/18 5:50 PM: > On 25/08/2018 11:00, Kai Wasserbäch wrote: >> The recent commit 4616639b49b4bbc91e503c1c27632dccc1c2b5be introduced >> the new function aubinator_error() which is a trivial wrapper around >> fprintf() to STDERR. The call to fprintf() however is passed the message >> msg directly: >> fprintf(stderr, msg); >> >> This is a format-security violation and leads to an FTBFS with >> -Werror=format-security (GCC 8): >> ../../../src/intel/tools/aubinator.c: In function 'aubinator_error': >> ../../../src/intel/tools/aubinator.c:74:4: error: format not a string >> literal and no format arguments [-Werror=format-security] >> fprintf(stderr, msg); >> ^~~ >> >> This patch fixes this trivially by introducing a catch-all "%s" format >> argument. >> >> Fixes: 4616639b49b ("intel: tools: split aub parsing from aubinator") >> Cc: Lionel Landwerlin >> Signed-off-by: Kai Wasserbäch > > > Reviewed-by: Lionel Landwerlin thank you for the review and pushing the patch for me! Cheers, Kai signature.asc Description: OpenPGP digital signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel: tools: Fix aubinator_error's fprintf call (format-security)
On 25/08/2018 11:00, Kai Wasserbäch wrote: The recent commit 4616639b49b4bbc91e503c1c27632dccc1c2b5be introduced the new function aubinator_error() which is a trivial wrapper around fprintf() to STDERR. The call to fprintf() however is passed the message msg directly: fprintf(stderr, msg); This is a format-security violation and leads to an FTBFS with -Werror=format-security (GCC 8): ../../../src/intel/tools/aubinator.c: In function 'aubinator_error': ../../../src/intel/tools/aubinator.c:74:4: error: format not a string literal and no format arguments [-Werror=format-security] fprintf(stderr, msg); ^~~ This patch fixes this trivially by introducing a catch-all "%s" format argument. Fixes: 4616639b49b ("intel: tools: split aub parsing from aubinator") Cc: Lionel Landwerlin Signed-off-by: Kai Wasserbäch Reviewed-by: Lionel Landwerlin --- Hey, just tried compiling the latest version of Mesa but due to commit 4616639b49b4bbc91e503c1c27632dccc1c2b5be by Lionel I got a FTBFS. Please commit this patch for me, if you accept it. I do not have commit access for Mesa. The pull request for this would be: The following changes since commit 1281608849a058fcdbe2f64481a159d58af3d888: gallium: Split out PIPE_CAP_TEXTURE_MIRROR_CLAMP_TO_EDGE. (2018-08-24 17:25:36 -0700) are available in the Git repository at: https://gitlab.freedesktop.org/curan/mesa.git aub-format-security for you to fetch changes up to 24957b0db3c12a097d70033ec8596da5bafedd55: intel: tools: Fix aubinator_error's fprintf call (format-security) (2018-08-25 11:43:25 +0200) Cheers, Kai src/intel/tools/aubinator.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/intel/tools/aubinator.c b/src/intel/tools/aubinator.c index 374ed46f86..c22d191f14 100644 --- a/src/intel/tools/aubinator.c +++ b/src/intel/tools/aubinator.c @@ -71,7 +71,7 @@ struct brw_instruction; static void aubinator_error(void *user_data, const void *aub_data, const char *msg) { - fprintf(stderr, msg); + fprintf(stderr, "%s", msg); } static void ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 6/9] nir: Make dead_write_vars pass global
On Sat, Aug 25, 2018 at 9:40 AM Jason Ekstrand wrote: > Sorry I haven't given you any in-line review yet. I've been letting this > pass simmer in my brain a bit and thinking about complexity and data > structures. Here's a few questions for which I do not expect you to have > actual answers: > > 1) How many unique derefs should we expect to see in a shader? Dozens? > Hundreds? Thousands? My guess is that, after vars_to_ssa has picked off > all the easy ones, it should be on the order of a couple hundred at most > but I don't actually know what a reasonable upper limit to assume is. > > 2) Is every deref going to be eventually compared to every other deref or > are we going to have separate little groups of derefs? > > I'm asking these questions because I have some concern about how expensive > this pass (and copy prop) is going to be. If you have a shader with a huge > number of loops each of which has a very self-contained set of derefs, the > current pass as-is probably has an efficiency that's around O(n) in the > number of instructions. However, in the case where most derefs are in play > all the time, then I supsect this pass as-is looks more like O(n^3) in the > number of derefs or worse. > > If the total number of unique derefs is reasonable AND the graph of deref > comparisons isn't too sparse, then I wonder if it wouldn't be better to > have some intermediate data structure like this: > > struct deref_compare_map { >void *mem_ctx; > >struct hash_table *id_map; > >nir_deref_path *paths; >BITSET_WORD **alias; >BITSET_WORD **contains; >BITSET_WORD **contained_in; > }; > > void deref_compare_map_init(struct deref_compare_map *map, void *mem_ctx) > { >memset(map, 0, sizeof(*map)); >map->mem_ctx = ralloc_context(mem_ctx); > >/* I'm pretty sure we have deref hashing and equality functions > somewhere already, we just have to pull them out and use them. It is, > however, possible that they were in nir_instr_set.c and got deleted as part > of the move to deref instructions. */ >map->id_map = _mesa_hash_table_create(map->mem_ctx, deref_hash, > deref_equal); > } > > void deref_compare_map_finish(struct deref_compare_map *map) > { >ralloc_free(map->mem_ctx); > } > > uint32_t deref_compare_map_get_id(struct deref_compare_map *map, > nir_deref_instr *deref) > { >struct hash_entry *entry = _mesa_hash_set_lookup(map->info_map, deref); >if (entry) > return (uint32_t)(uintptr_t)entry->data; > >/* We can't be adding anything after we've baked it */ >assert(map->alias == NULL); > >uint32_t id = map->id_map->entries; >_mesa_hash_set_add(map->id_map, deref, id); >assert(map->id_map->entries == id + 1); > } > > static BITSET_WORDS ** > alloc_set_of_sets(void *mem_ctx, unsigned entries) > { >BITSET_WORDS **sets = ralloc_array(mem_ctx, BITSET_WORD *, entries); >BITSET_WORDS *data = rzalloc_array(mem_ctx, BITSET_WORD, entries * > BITSET_WORDS(entries)); >for (unsigned i = 0; i < entries; i++) > sets[i] = data + i * BITSET_WORDS(entries); >return sets; > } > > void deref_compare_map_bake(struct deref_compare_map *map) > { >const unsigned num_derefs = map->id_map->entries; > >map->paths = ralloc_array(map->mem_ctx, nir_deref_path, num_derefs); >struct hash_entry *entry; >hash_table_foreach(map->id_map, entry) { > uint32_t id = (uintptr_t)entry->data; > nir_deref_path_init(>paths[id], (void *)entry->key, > map->mem_ctx); >} > >map->alias = alloc_set_of_sets(map->mem_ctx, map->id_map->entries); >map->contains = alloc_set_of_sets(map->mem_ctx, map->id_map->entries); >map->contained_in = alloc_set_of_sets(map->mem_ctx, > map->id_map->entries); > >for (unsigned a = 0; a < num_derefs; a++) { > /* Handle comparing a with itself */ > BITSET_SET(map->alias[a], a); > BITSET_SET(map->contains[a], a); > BITSET_SET(map->contained_in[a], a); > > for (unsigned b = 0; b < a; b++) { > enum nir_deref_compare_result comp = > nir_compare_deref_paths(>paths[a], >paths[b]); > if (comp & derefs_may_alias_bit) { > BITSET_SET(map->alias[a], b); > BITSET_SET(map->alias[b], a); > } > > if (comp & derefs_a_contains_b_bit) { > BITSET_SET(map->contains[a], b); > BITSET_SET(map->contained_in[b], a); > } > > if (comp & derefs_b_contains_a_bit) { > BITSET_SET(map->contains[b], a); > BITSET_SET(map->contained_in[a], b); > } > } >} > } > > The idea would be to make it a 5-step pass: > > 1) find all derefs > 2) Bake the deref compare map > 3) Local elimination and building kill etc. sets. > 4) Data-flow > 5) global elimination > > Doing that would give us some better run-time guarantees: > > 1) Set operations on bitsets are O(n) (where n is actually > DIV_ROUND_UP(n, 32) thanks to bitsets) > 2) We call
Re: [Mesa-dev] [PATCH] st/mesa: Override blend factors involving alpha if it doesn't exist.
You have to make sure to flip blend->independent_blend_enable to 1 in that case, and that will only work on some (well, most, but not all) hardware. Pre-something Tesla-era and I assume r600-era gpu's must have all rt's configured the same way. I'm also not 100% sure that it's OK to access the draw buffers in here, but Marek would know for sure -- he restructured the atoms logic a while back, and I haven't re-learned the new scheme. -ilia On Sat, Aug 25, 2018 at 3:48 AM, Kenneth Graunke wrote: > When faking an RGB format with an RGBA format, there may be a channel > of data containing garbage. st/mesa already overrides texture swizzles > to replace the A channel with ONE. This patch makes it override blend > factors to achieve a similar effect. > > It appears that st_update_blend is already called when the framebuffer > is changed, so it should be safe to look at the render target formats > without adding additional state dependencies. > --- > src/mesa/state_tracker/st_atom_blend.c | 35 ++ > 1 file changed, 35 insertions(+) > > diff --git a/src/mesa/state_tracker/st_atom_blend.c > b/src/mesa/state_tracker/st_atom_blend.c > index 804de2f154f..3e6e4411107 100644 > --- a/src/mesa/state_tracker/st_atom_blend.c > +++ b/src/mesa/state_tracker/st_atom_blend.c > @@ -41,6 +41,7 @@ > > #include "framebuffer.h" > #include "main/blend.h" > +#include "main/glformats.h" > #include "main/macros.h" > > /** > @@ -142,6 +143,28 @@ blend_per_rt(const struct gl_context *ctx, unsigned > num_cb) > return GL_FALSE; > } > > +/** > + * Modify blend function to force destination alpha to 1.0 > + * > + * If \c function specifies a blend function that uses destination alpha, > + * replace it with a function that hard-wires destination alpha to 1.0. This > + * is used when rendering to xRGB targets. > + */ > +static enum pipe_blendfactor > +fix_xrgb_alpha(enum pipe_blendfactor factor) > +{ > + switch (factor) { > + case PIPE_BLENDFACTOR_DST_ALPHA: > + return PIPE_BLENDFACTOR_ONE; > + > + case PIPE_BLENDFACTOR_INV_DST_ALPHA: > + case PIPE_BLENDFACTOR_SRC_ALPHA_SATURATE: > + return PIPE_BLENDFACTOR_ZERO; > + default: > + return factor; > + } > +} > + > void > st_update_blend( struct st_context *st ) > { > @@ -210,6 +233,18 @@ st_update_blend( struct st_context *st ) > blend->rt[i].alpha_dst_factor = > translate_blend(ctx->Color.Blend[j].DstA); > } > + > + const struct gl_renderbuffer *rb = > +ctx->DrawBuffer->_ColorDrawBuffers[i]; > + > + if (rb && !_mesa_base_format_has_channel(rb->_BaseFormat, > + GL_TEXTURE_ALPHA_TYPE)) { > +struct pipe_rt_blend_state *rt = >rt[i]; > +rt->rgb_src_factor = fix_xrgb_alpha(rt->rgb_src_factor); > +rt->rgb_dst_factor = fix_xrgb_alpha(rt->rgb_dst_factor); > +rt->alpha_src_factor = fix_xrgb_alpha(rt->alpha_src_factor); > +rt->alpha_dst_factor = fix_xrgb_alpha(rt->alpha_dst_factor); > + } >} > } > else { > -- > 2.18.0 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 6/9] nir: Make dead_write_vars pass global
Sorry I haven't given you any in-line review yet. I've been letting this pass simmer in my brain a bit and thinking about complexity and data structures. Here's a few questions for which I do not expect you to have actual answers: 1) How many unique derefs should we expect to see in a shader? Dozens? Hundreds? Thousands? My guess is that, after vars_to_ssa has picked off all the easy ones, it should be on the order of a couple hundred at most but I don't actually know what a reasonable upper limit to assume is. 2) Is every deref going to be eventually compared to every other deref or are we going to have separate little groups of derefs? I'm asking these questions because I have some concern about how expensive this pass (and copy prop) is going to be. If you have a shader with a huge number of loops each of which has a very self-contained set of derefs, the current pass as-is probably has an efficiency that's around O(n) in the number of instructions. However, in the case where most derefs are in play all the time, then I supsect this pass as-is looks more like O(n^3) in the number of derefs or worse. If the total number of unique derefs is reasonable AND the graph of deref comparisons isn't too sparse, then I wonder if it wouldn't be better to have some intermediate data structure like this: struct deref_compare_map { void *mem_ctx; struct hash_table *id_map; nir_deref_path *paths; BITSET_WORD **alias; BITSET_WORD **contains; BITSET_WORD **contained_in; }; void deref_compare_map_init(struct deref_compare_map *map, void *mem_ctx) { memset(map, 0, sizeof(*map)); map->mem_ctx = ralloc_context(mem_ctx); /* I'm pretty sure we have deref hashing and equality functions somewhere already, we just have to pull them out and use them. It is, however, possible that they were in nir_instr_set.c and got deleted as part of the move to deref instructions. */ map->id_map = _mesa_hash_table_create(map->mem_ctx, deref_hash, deref_equal); } void deref_compare_map_finish(struct deref_compare_map *map) { ralloc_free(map->mem_ctx); } uint32_t deref_compare_map_get_id(struct deref_compare_map *map, nir_deref_instr *deref) { struct hash_entry *entry = _mesa_hash_set_lookup(map->info_map, deref); if (entry) return (uint32_t)(uintptr_t)entry->data; /* We can't be adding anything after we've baked it */ assert(map->alias == NULL); uint32_t id = map->id_map->entries; _mesa_hash_set_add(map->id_map, deref, id); assert(map->id_map->entries == id + 1); } static BITSET_WORDS ** alloc_set_of_sets(void *mem_ctx, unsigned entries) { BITSET_WORDS **sets = ralloc_array(mem_ctx, BITSET_WORD *, entries); BITSET_WORDS *data = rzalloc_array(mem_ctx, BITSET_WORD, entries * BITSET_WORDS(entries)); for (unsigned i = 0; i < entries; i++) sets[i] = data + i * BITSET_WORDS(entries); return sets; } void deref_compare_map_bake(struct deref_compare_map *map) { const unsigned num_derefs = map->id_map->entries; map->paths = ralloc_array(map->mem_ctx, nir_deref_path, num_derefs); struct hash_entry *entry; hash_table_foreach(map->id_map, entry) { uint32_t id = (uintptr_t)entry->data; nir_deref_path_init(>paths[id], (void *)entry->key, map->mem_ctx); } map->alias = alloc_set_of_sets(map->mem_ctx, map->id_map->entries); map->contains = alloc_set_of_sets(map->mem_ctx, map->id_map->entries); map->contained_in = alloc_set_of_sets(map->mem_ctx, map->id_map->entries); for (unsigned a = 0; a < num_derefs; a++) { /* Handle comparing a with itself */ BITSET_SET(map->alias[a], a); BITSET_SET(map->contains[a], a); BITSET_SET(map->contained_in[a], a); for (unsigned b = 0; b < a; b++) { enum nir_deref_compare_result comp = nir_compare_deref_paths(>paths[a], >paths[b]); if (comp & derefs_may_alias_bit) { BITSET_SET(map->alias[a], b); BITSET_SET(map->alias[b], a); } if (comp & derefs_a_contains_b_bit) { BITSET_SET(map->contains[a], b); BITSET_SET(map->contained_in[b], a); } if (comp & derefs_b_contains_a_bit) { BITSET_SET(map->contains[b], a); BITSET_SET(map->contained_in[a], b); } } } } The idea would be to make it a 5-step pass: 1) find all derefs 2) Bake the deref compare map 3) Local elimination and building kill etc. sets. 4) Data-flow 5) global elimination Doing that would give us some better run-time guarantees: 1) Set operations on bitsets are O(n) (where n is actually DIV_ROUND_UP(n, 32) thanks to bitsets) 2) We call nir_deref_path_init exactly num_derefs times for the entire pass 3) We call nir_compare_deref_paths exactly (num_derefs * (num_derefs - 1)) / 2 times for the entire pass. As it is, deref_map_add_map is O(n^2) comparisons and we do that operation inside our fixed-point loop which has an unknown number of
Re: [Mesa-dev] [PATCH 2/9] nir: Export deref comparison functions
On Wed, Aug 15, 2018 at 4:57 PM Caio Marcelo de Oliveira Filho < caio.olive...@intel.com> wrote: > Reviewed-by: Timothy Arceri > --- > src/compiler/nir/nir_deref.c | 109 > src/compiler/nir/nir_deref.h | 10 ++ > src/compiler/nir/nir_opt_copy_prop_vars.c | 145 ++ > 3 files changed, 132 insertions(+), 132 deletions(-) > > diff --git a/src/compiler/nir/nir_deref.c b/src/compiler/nir/nir_deref.c > index c03acf83597..d013b423a8b 100644 > --- a/src/compiler/nir/nir_deref.c > +++ b/src/compiler/nir/nir_deref.c > @@ -270,3 +270,112 @@ nir_fixup_deref_modes(nir_shader *shader) >} > } > } > + > +/** Returns true if the storage referrenced to by deref completely > contains > + * the storage referenced by sub. > Please fix this comment while we're here. It's so out of date as to be laughable. > + */ > +nir_deref_compare_result > +nir_compare_deref_paths(nir_deref_path *a_path, > +nir_deref_path *b_path) > +{ > + if (a_path->path[0]->var != b_path->path[0]->var) > + return 0; > + > + /* Start off assuming they fully compare. We ignore equality for > now. In > +* the end, we'll determine that by containment. > +*/ > + nir_deref_compare_result result = nir_derefs_may_alias_bit | > + nir_derefs_a_contains_b_bit | > + nir_derefs_b_contains_a_bit; > + > + nir_deref_instr **a_p = _path->path[1]; > + nir_deref_instr **b_p = _path->path[1]; > + while (*a_p != NULL && *b_p != NULL) { > + nir_deref_instr *a_tail = *(a_p++); > + nir_deref_instr *b_tail = *(b_p++); > + > + switch (a_tail->deref_type) { > + case nir_deref_type_array: > + case nir_deref_type_array_wildcard: { > + assert(b_tail->deref_type == nir_deref_type_array || > +b_tail->deref_type == nir_deref_type_array_wildcard); > + > + if (a_tail->deref_type == nir_deref_type_array_wildcard) { > +if (b_tail->deref_type != nir_deref_type_array_wildcard) > + result &= ~nir_derefs_b_contains_a_bit; > + } else if (b_tail->deref_type == nir_deref_type_array_wildcard) { > +if (a_tail->deref_type != nir_deref_type_array_wildcard) > + result &= ~nir_derefs_a_contains_b_bit; > + } else { > +assert(a_tail->deref_type == nir_deref_type_array && > + b_tail->deref_type == nir_deref_type_array); > +assert(a_tail->arr.index.is_ssa && b_tail->arr.index.is_ssa); > + > +nir_const_value *a_index_const = > + nir_src_as_const_value(a_tail->arr.index); > +nir_const_value *b_index_const = > + nir_src_as_const_value(b_tail->arr.index); > +if (a_index_const && b_index_const) { > + /* If they're both direct and have different offsets, they > +* don't even alias much less anything else. > +*/ > + if (a_index_const->u32[0] != b_index_const->u32[0]) > + return 0; > +} else if (a_tail->arr.index.ssa == b_tail->arr.index.ssa) { > + /* They're the same indirect, continue on */ > +} else { > + /* They're not the same index so we can't prove anything > about > +* containment. > +*/ > + result &= ~(nir_derefs_a_contains_b_bit | > nir_derefs_b_contains_a_bit); > +} > + } > + break; > + } > + > + case nir_deref_type_struct: { > + /* If they're different struct members, they don't even alias */ > + if (a_tail->strct.index != b_tail->strct.index) > +return 0; > + break; > + } > + > + default: > + unreachable("Invalid deref type"); > + } > + } > + > + /* If a is longer than b, then it can't contain b */ > + if (*a_p != NULL) > + result &= ~nir_derefs_a_contains_b_bit; > + if (*b_p != NULL) > + result &= ~nir_derefs_b_contains_a_bit; > + > + /* If a contains b and b contains a they must be equal. */ > + if ((result & nir_derefs_a_contains_b_bit) && (result & > nir_derefs_b_contains_a_bit)) > + result |= nir_derefs_equal_bit; > + > + return result; > +} > + > +nir_deref_compare_result > +nir_compare_derefs(nir_deref_instr *a, nir_deref_instr *b) > +{ > + if (a == b) { > + return nir_derefs_equal_bit | nir_derefs_may_alias_bit | > + nir_derefs_a_contains_b_bit | nir_derefs_b_contains_a_bit; > + } > + > + nir_deref_path a_path, b_path; > + nir_deref_path_init(_path, a, NULL); > + nir_deref_path_init(_path, b, NULL); > + assert(a_path.path[0]->deref_type == nir_deref_type_var); > + assert(b_path.path[0]->deref_type == nir_deref_type_var); > + > + nir_deref_compare_result result = nir_compare_deref_paths(_path, > _path); > + > +
Re: [Mesa-dev] [PATCH 2/3] nir: Create sampler variables in prog_to_nir.
On Fri, Aug 24, 2018 at 8:24 PM Kenneth Graunke wrote: > This is needed for nir_gather_info to actually count the textures, > since it operates solely on variables. > --- > src/mesa/program/prog_to_nir.c | 15 +-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/src/mesa/program/prog_to_nir.c > b/src/mesa/program/prog_to_nir.c > index 14e57b6c6a1..1f0607542e8 100644 > --- a/src/mesa/program/prog_to_nir.c > +++ b/src/mesa/program/prog_to_nir.c > @@ -52,6 +52,7 @@ struct ptn_compile { > nir_variable *parameters; > nir_variable *input_vars[VARYING_SLOT_MAX]; > nir_variable *output_vars[VARYING_SLOT_MAX]; > + nir_variable *sampler_vars[32]; /* matches number of bits in > TexSrcUnit */ > nir_register **output_regs; > nir_register **temp_regs; > > @@ -484,9 +485,10 @@ ptn_kil(nir_builder *b, nir_ssa_def **src) > } > > static void > -ptn_tex(nir_builder *b, nir_alu_dest dest, nir_ssa_def **src, > +ptn_tex(struct ptn_compile *c, nir_alu_dest dest, nir_ssa_def **src, > struct prog_instruction *prog_inst) > { > + nir_builder *b = >build; > nir_tex_instr *instr; > nir_texop op; > unsigned num_srcs; > @@ -568,6 +570,15 @@ ptn_tex(nir_builder *b, nir_alu_dest dest, > nir_ssa_def **src, >unreachable("can't reach"); > } > > + if (!c->sampler_vars[prog_inst->TexSrcUnit]) { > + const struct glsl_type *type = > + glsl_sampler_type(instr->sampler_dim, false, false, > GLSL_TYPE_FLOAT); > + nir_variable *var = > + nir_variable_create(b->shader, nir_var_uniform, type, "sampler"); > + var->data.binding = prog_inst->TexSrcUnit; > + c->sampler_vars[prog_inst->TexSrcUnit] = var; > Can samplers be indirected? If so, we probably want an array of samplers instead 32 distinct samplers. > + } > + > unsigned src_number = 0; > > instr->src[src_number].src = > @@ -784,7 +795,7 @@ ptn_emit_instruction(struct ptn_compile *c, struct > prog_instruction *prog_inst) > case OPCODE_TXD: > case OPCODE_TXL: > case OPCODE_TXP: > - ptn_tex(b, dest, src, prog_inst); > + ptn_tex(c, dest, src, prog_inst); >break; > > case OPCODE_SWZ: > -- > 2.18.0 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel: tools: Fix aubinator_error's fprintf call (format-security)
The recent commit 4616639b49b4bbc91e503c1c27632dccc1c2b5be introduced the new function aubinator_error() which is a trivial wrapper around fprintf() to STDERR. The call to fprintf() however is passed the message msg directly: fprintf(stderr, msg); This is a format-security violation and leads to an FTBFS with -Werror=format-security (GCC 8): ../../../src/intel/tools/aubinator.c: In function 'aubinator_error': ../../../src/intel/tools/aubinator.c:74:4: error: format not a string literal and no format arguments [-Werror=format-security] fprintf(stderr, msg); ^~~ This patch fixes this trivially by introducing a catch-all "%s" format argument. Fixes: 4616639b49b ("intel: tools: split aub parsing from aubinator") Cc: Lionel Landwerlin Signed-off-by: Kai Wasserbäch --- Hey, just tried compiling the latest version of Mesa but due to commit 4616639b49b4bbc91e503c1c27632dccc1c2b5be by Lionel I got a FTBFS. Please commit this patch for me, if you accept it. I do not have commit access for Mesa. The pull request for this would be: The following changes since commit 1281608849a058fcdbe2f64481a159d58af3d888: gallium: Split out PIPE_CAP_TEXTURE_MIRROR_CLAMP_TO_EDGE. (2018-08-24 17:25:36 -0700) are available in the Git repository at: https://gitlab.freedesktop.org/curan/mesa.git aub-format-security for you to fetch changes up to 24957b0db3c12a097d70033ec8596da5bafedd55: intel: tools: Fix aubinator_error's fprintf call (format-security) (2018-08-25 11:43:25 +0200) Cheers, Kai src/intel/tools/aubinator.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/intel/tools/aubinator.c b/src/intel/tools/aubinator.c index 374ed46f86..c22d191f14 100644 --- a/src/intel/tools/aubinator.c +++ b/src/intel/tools/aubinator.c @@ -71,7 +71,7 @@ struct brw_instruction; static void aubinator_error(void *user_data, const void *aub_data, const char *msg) { - fprintf(stderr, msg); + fprintf(stderr, "%s", msg); } static void -- 2.18.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] st/mesa: Override blend factors involving alpha if it doesn't exist.
When faking an RGB format with an RGBA format, there may be a channel of data containing garbage. st/mesa already overrides texture swizzles to replace the A channel with ONE. This patch makes it override blend factors to achieve a similar effect. It appears that st_update_blend is already called when the framebuffer is changed, so it should be safe to look at the render target formats without adding additional state dependencies. --- src/mesa/state_tracker/st_atom_blend.c | 35 ++ 1 file changed, 35 insertions(+) diff --git a/src/mesa/state_tracker/st_atom_blend.c b/src/mesa/state_tracker/st_atom_blend.c index 804de2f154f..3e6e4411107 100644 --- a/src/mesa/state_tracker/st_atom_blend.c +++ b/src/mesa/state_tracker/st_atom_blend.c @@ -41,6 +41,7 @@ #include "framebuffer.h" #include "main/blend.h" +#include "main/glformats.h" #include "main/macros.h" /** @@ -142,6 +143,28 @@ blend_per_rt(const struct gl_context *ctx, unsigned num_cb) return GL_FALSE; } +/** + * Modify blend function to force destination alpha to 1.0 + * + * If \c function specifies a blend function that uses destination alpha, + * replace it with a function that hard-wires destination alpha to 1.0. This + * is used when rendering to xRGB targets. + */ +static enum pipe_blendfactor +fix_xrgb_alpha(enum pipe_blendfactor factor) +{ + switch (factor) { + case PIPE_BLENDFACTOR_DST_ALPHA: + return PIPE_BLENDFACTOR_ONE; + + case PIPE_BLENDFACTOR_INV_DST_ALPHA: + case PIPE_BLENDFACTOR_SRC_ALPHA_SATURATE: + return PIPE_BLENDFACTOR_ZERO; + default: + return factor; + } +} + void st_update_blend( struct st_context *st ) { @@ -210,6 +233,18 @@ st_update_blend( struct st_context *st ) blend->rt[i].alpha_dst_factor = translate_blend(ctx->Color.Blend[j].DstA); } + + const struct gl_renderbuffer *rb = +ctx->DrawBuffer->_ColorDrawBuffers[i]; + + if (rb && !_mesa_base_format_has_channel(rb->_BaseFormat, + GL_TEXTURE_ALPHA_TYPE)) { +struct pipe_rt_blend_state *rt = >rt[i]; +rt->rgb_src_factor = fix_xrgb_alpha(rt->rgb_src_factor); +rt->rgb_dst_factor = fix_xrgb_alpha(rt->rgb_dst_factor); +rt->alpha_src_factor = fix_xrgb_alpha(rt->alpha_src_factor); +rt->alpha_dst_factor = fix_xrgb_alpha(rt->alpha_dst_factor); + } } } else { -- 2.18.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] meson: Be a bit more helpful when arch or OS is unknown
On Fri, 24 Aug 2018 at 22:23, Guido Günther wrote: > > @@ -230,7 +236,8 @@ if _platforms.contains('auto') >elif ['haiku'].contains(host_machine.system()) > _platforms = ['haiku'] >else > -error('Unknown OS. Please pass -Dplatforms to set platforms. Patches > gladly accepted to fix this.') > +error('Unknown OS. Please pass -Dplatforms to set platforms. Patches > gladly accepted to fix this.'.format( > + host_machine.system())) >endif > endif > Did you mean that last "Unknown OS." to be "Unknown OS @0@." like all the others? -- Stuart Young (aka Cefiar) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] st/mesa: Disable blending for integer formats.
Blending isn't valid for integer formats. Rather than having drivers worry about this, just disable blending in this case. This hopefully will increase hits in the CSO cache as well, by eliminating most of the meaningless fields in this case. --- src/mesa/state_tracker/st_atom_blend.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/mesa/state_tracker/st_atom_blend.c b/src/mesa/state_tracker/st_atom_blend.c index 57400e2e791..804de2f154f 100644 --- a/src/mesa/state_tracker/st_atom_blend.c +++ b/src/mesa/state_tracker/st_atom_blend.c @@ -171,6 +171,7 @@ st_update_blend( struct st_context *st ) /* blending enabled */ for (i = 0, j = 0; i < num_state; i++) { if (!(ctx->Color.BlendEnabled & (1 << i)) || + (ctx->DrawBuffer->_IntegerBuffers & (1 << i)) || !blend->rt[i].colormask) continue; -- 2.18.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/5] Emit BRW_AOP_INC or BRW_AOP_DEC
Ian Romanick writes: > I don't know why we never did this. Almost every shader in shader-db > that uses atomicAdd or imageAtomicAdd uses it with a constant of 1 or > -1. Nice. The series is Reviewed-by: Caio Marcelo de Oliveira Filho Thanks, Caio ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] run: Add Cannonlake and Ice Lake as platforms
From: Ian Romanick PCI IDs directly from Mesa's include/pci_ids/i965_pci_ids.h. Signed-off-by: Ian Romanick --- run.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/run.c b/run.c index 200c0f4..c70e367 100644 --- a/run.c +++ b/run.c @@ -418,6 +418,8 @@ const struct platform platforms[] = { "byt", "0x0F33", "bdw", "0x162E", "skl", "0x191D", +"cnl", "0x5A54", +"icl", "0x8A50", }; void print_usage(const char *prog_name) -- 2.14.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] report.py: Gather and log some statistics about the helped / hurt data
From: Ian Romanick A previous method that I tried was treating the helped and hurt data as samples from separate populations, and these were compared using a T-test. Since we're applying a common change to "both" sample sets, I don't think this is a valid analysis. Instead, I think it is more valid to look at the entire change set as a sample of a single population and compare the mean of that sample to zero. Only the changed samples are examined because the vast majority of the sample in unaffected. If the mean of the entire sample was used, the mean confidence interval would always include zero. It would be more valid, I believe, include shaders that were affected but had no change in instruction or cycle count. I don't know of a way to determine this using the existing shader-db infrastructure. These two different methods communicate two different things. The first tries to determine whether the shaders hurt are affected more or less than the shaders helped. This doesn't capture any information about the number of shaders affected. There might be 1,000 shaders helped and 3 hurt, and the conclusion could still be negative. The second methods trieds to determine whether the sample set is overall helped or hurt. This allows the magnitued of hurt (or help) to be overwhelmed by the number of helped (or hurt) shaders. There could be 1,000 shaders helped by 1 instruction and 3 shaders hurt by 50 instructions, and the conclusion would be positive. Comparing the declared result with the mean and median, I feel like the second method matches my intuitive interpretation of the data. Here is a result of the T-test: total cycles in shared programs: 559379982 -> 559342256 (<.01%) cycles in affected programs: 10791218 -> 10753492 (-0.35%) helped: 1952 HURT: 908 helped stats (abs) min: 1 max: 5762 x̄: 37.71 x̃: 16 helped stats (rel) min: <.01% max: 28.57% x̄: 3.54% x̃: 2.09% HURT stats (abs) min: 1 max: 573 x̄: 39.51 x̃: 10 HURT stats (rel) min: <.01% max: 27.78% x̄: 1.93% x̃: 0.66% abs t: -0.34, p: 73.70% rel t: 9.88, p: <.01% Inconclusive result (cannot disprove both null hypothoses). And here is the result of the mean confidence interval tests on the same data: total cycles in shared programs: 559378112 -> 559340386 (<.01%) cycles in affected programs: 10791218 -> 10753492 (-0.35%) helped: 1952 HURT: 908 helped stats (abs) min: 1 max: 5762 x̄: 37.71 x̃: 16 helped stats (rel) min: <.01% max: 28.57% x̄: 3.54% x̃: 2.09% HURT stats (abs) min: 1 max: 573 x̄: 39.51 x̃: 10 HURT stats (rel) min: <.01% max: 27.78% x̄: 1.93% x̃: 0.66% 95% mean confidence interval for cycles value: -18.27 -8.11 95% mean confidence interval for cycles %-change: -1.98% -1.63% Cycles are helped. Since the confidence interval is calculated based on the sample mean and the sample standard deviation, it can include values out side the sample minimum and maximum. This can lead to unexpected conclusions. In this case all of the affected shaders were helped, but the result is inconclusive. total instructions in shared programs: 7886959 -> 7886925 (<.01%) instructions in affected programs: 1340 -> 1306 (-2.54%) helped: 4 HURT: 0 helped stats (abs) min: 2 max: 15 x̄: 8.50 x̃: 8 helped stats (rel) min: 0.63% max: 4.30% x̄: 2.45% x̃: 2.43% 95% mean confidence interval for instructions value: -20.44 3.44 95% mean confidence interval for instructions %-change: -5.78% 0.89% Inconclusive result (value mean confidence interval includes 0). v2: Don't log statistics for spill or fills. Simplify T-test logging. v3: Use confidence interval instead. Signed-off-by: Ian Romanick --- report.py | 101 +- 1 file changed, 100 insertions(+), 1 deletion(-) diff --git a/report.py b/report.py index b306220..6d53052 100755 --- a/report.py +++ b/report.py @@ -2,6 +2,9 @@ import re import argparse +import statistics +from scipy import stats +import numpy as np def get_results(filename): @@ -63,6 +66,35 @@ def get_result_string(p, b, a): def split_list(string): return string.split(",") + +def gather_statistics(changes, before, after, m): +stats = (0.0, 0, 0.0, 0, 0, 0.0, 0.0, 0.0, 0.0, 0.0) + +num = len(changes) +if num > 0: +absolute = [abs(before[p][m] - after[p][m]) for p in changes] +relative = [0 if before[p][m] == 0 else abs(before[p][m] - after[p][m]) / before[p][m] for p in changes] + +stats = (statistics.mean(absolute), + statistics.median(absolute), + min(absolute), + max(absolute), + statistics.mean(relative), + statistics.median(relative), + min(relative), + max(relative)) + +return stats + + +def mean_confidence_interval(data, confidence=0.95): +a = 1.0 * np.array(data) +n