Re: [Mesa-dev] [PATCH 16/16] i965: Remove the brw_context from the visitors
For the series:
Reviewed-by: Chris Forbes
On Tue, Jun 23, 2015 at 1:07 PM, Jason Ekstrand wrote:
> As of this commit, nothing actually needs the brw_context.
> ---
> src/mesa/drivers/dri/i965/brw_cs.cpp| 6 --
> src/mesa/drivers/dri/i965/brw_fs.cpp| 12 ++--
> src/mesa/drivers/dri/i965/brw_fs.h | 2 +-
> src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp | 1 -
> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp| 4 ++--
> src/mesa/drivers/dri/i965/brw_shader.cpp| 9 +
> src/mesa/drivers/dri/i965/brw_shader.h | 7 ---
> src/mesa/drivers/dri/i965/brw_vec4.cpp | 6 --
> src/mesa/drivers/dri/i965/brw_vec4.h| 2 +-
> src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp | 14 --
> src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.h | 2 +-
> src/mesa/drivers/dri/i965/brw_vec4_reg_allocate.cpp | 1 -
> src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 4 ++--
> src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp | 4 ++--
> src/mesa/drivers/dri/i965/brw_vs.h | 2 +-
> src/mesa/drivers/dri/i965/gen6_gs_visitor.h | 4 ++--
> 16 files changed, 43 insertions(+), 37 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_cs.cpp
> b/src/mesa/drivers/dri/i965/brw_cs.cpp
> index fa8b5c8..4c5082c 100644
> --- a/src/mesa/drivers/dri/i965/brw_cs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_cs.cpp
> @@ -94,7 +94,8 @@ brw_cs_emit(struct brw_context *brw,
>
> /* Now the main event: Visit the shader IR and generate our CS IR for it.
> */
> - fs_visitor v8(brw, mem_ctx, MESA_SHADER_COMPUTE, key, &prog_data->base,
> prog,
> + fs_visitor v8(brw->intelScreen->compiler, brw,
> + mem_ctx, MESA_SHADER_COMPUTE, key, &prog_data->base, prog,
> &cp->Base, 8, st_index);
> if (!v8.run_cs()) {
>fail_msg = v8.fail_msg;
> @@ -103,7 +104,8 @@ brw_cs_emit(struct brw_context *brw,
>prog_data->simd_size = 8;
> }
>
> - fs_visitor v16(brw, mem_ctx, MESA_SHADER_COMPUTE, key, &prog_data->base,
> prog,
> + fs_visitor v16(brw->intelScreen->compiler, brw,
> + mem_ctx, MESA_SHADER_COMPUTE, key, &prog_data->base, prog,
>&cp->Base, 16, st_index);
> if (likely(!(INTEL_DEBUG & DEBUG_NO16)) &&
> !fail_msg && !v8.simd16_unsupported &&
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 23f60c2..f7f05af 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -677,8 +677,7 @@ fs_visitor::no16(const char *msg)
> } else {
>simd16_unsupported = true;
>
> - struct brw_compiler *compiler = brw->intelScreen->compiler;
> - compiler->shader_perf_log(brw,
> + compiler->shader_perf_log(log_data,
> "SIMD16 shader failed to compile: %s", msg);
> }
> }
> @@ -3757,8 +3756,7 @@ fs_visitor::allocate_registers()
> fail("Failure to register allocate. Reduce number of "
>"live scalar values to avoid this.");
>} else {
> - struct brw_compiler *compiler = brw->intelScreen->compiler;
> - compiler->shader_perf_log(brw,
> + compiler->shader_perf_log(log_data,
> "%s shader triggered register spilling. "
> "Try reducing the number of live scalar "
> "values to improve performance.\n",
> @@ -3994,7 +3992,8 @@ brw_wm_fs_emit(struct brw_context *brw,
>
> /* Now the main event: Visit the shader IR and generate our FS IR for it.
> */
> - fs_visitor v(brw, mem_ctx, MESA_SHADER_FRAGMENT, key, &prog_data->base,
> + fs_visitor v(brw->intelScreen->compiler, brw,
> +mem_ctx, MESA_SHADER_FRAGMENT, key, &prog_data->base,
> prog, &fp->Base, 8, st_index8);
> if (!v.run_fs(false /* do_rep_send */)) {
>if (prog) {
> @@ -4009,7 +4008,8 @@ brw_wm_fs_emit(struct brw_context *brw,
> }
>
> cfg_t *simd16_cfg = NULL;
> - fs_visitor v2(brw, mem_ctx, MESA_SHADER_FRAGMENT, key, &prog_data->base,
> + fs_visitor v2(brw->intelScreen->compiler, brw,
> + mem_ctx, MESA_SHADER_FRAGMENT, key, &prog_data->base,
> prog, &fp->Base, 16, st_index16);
> if (likely(!(INTEL_DEBUG & DEBUG_NO16) || brw->use_rep_send)) {
>if (!v.simd16_unsupported) {
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
> b/src/mesa/drivers/dri/i965/brw_fs.h
> index e0a8984..243baf6 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -70,7 +70,7 @@ namespace brw {
> class fs_visitor : public backend_shader
> {
> public:
> - fs_visitor(struct brw_context *brw,
> + fs_visitor(const struct brw_compiler *compiler, void *log_data,
>void
Re: [Mesa-dev] [Nouveau] [RFC PATCH 5/8] nv50: prevent NULL pointer dereference with pipe_query functions
On 06/23/2015 08:57 AM, Michel Dänzer wrote: On 23.06.2015 06:02, Samuel Pitoiset wrote: On 06/22/2015 10:52 PM, Ilia Mirkin wrote: If query_create fails, why would any of these functions get called? Because the HUD doesn't check if query_create() fails and it calls other pipe_query functions with NULL pointer instead of a valid query object. Could the HUD code be fixed instead? It's definitely possible, and probably the best solution instead of preventing NULL pointer dereference in the underlying drivers. I'll make a patch. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3] glsl/es31:Allow GL_ARB_TEXTURE_MULTISAMPLE in GLSL ES 3.10
From: Marta Lofstedt
Signed-off-by: Marta Lofstedt
---
src/glsl/builtin_functions.cpp | 3 +--
src/glsl/builtin_types.cpp | 2 +-
src/glsl/glsl_lexer.ll | 13 +++--
src/glsl/glsl_parser_extras.h | 5 +
4 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp
index efab299..593a575 100644
--- a/src/glsl/builtin_functions.cpp
+++ b/src/glsl/builtin_functions.cpp
@@ -270,8 +270,7 @@ texture_array(const _mesa_glsl_parse_state *state)
static bool
texture_multisample(const _mesa_glsl_parse_state *state)
{
- return state->is_version(150, 0) ||
- state->ARB_texture_multisample_enable;
+ return state->has_texture_multisample();
}
static bool
diff --git a/src/glsl/builtin_types.cpp b/src/glsl/builtin_types.cpp
index d92e2eb..9968f7c 100644
--- a/src/glsl/builtin_types.cpp
+++ b/src/glsl/builtin_types.cpp
@@ -307,7 +307,7 @@ _mesa_glsl_initialize_types(struct _mesa_glsl_parse_state
*state)
add_type(symbols, glsl_type::usamplerCubeArray_type);
}
- if (state->ARB_texture_multisample_enable) {
+ if (state->has_texture_multisample()) {
add_type(symbols, glsl_type::sampler2DMS_type);
add_type(symbols, glsl_type::isampler2DMS_type);
add_type(symbols, glsl_type::usampler2DMS_type);
diff --git a/src/glsl/glsl_lexer.ll b/src/glsl/glsl_lexer.ll
index 10db5b8..3597435 100644
--- a/src/glsl/glsl_lexer.ll
+++ b/src/glsl/glsl_lexer.ll
@@ -341,12 +341,13 @@ usampler2DArray KEYWORD(130, 300, 130, 300,
USAMPLER2DARRAY);
/* additional keywords in ARB_texture_multisample, included in GLSL 1.50 */
/* these are reserved but not defined in GLSL 3.00 */
-sampler2DMSKEYWORD_WITH_ALT(150, 300, 150, 0,
yyextra->ARB_texture_multisample_enable, SAMPLER2DMS);
-isampler2DMS KEYWORD_WITH_ALT(150, 300, 150, 0,
yyextra->ARB_texture_multisample_enable, ISAMPLER2DMS);
-usampler2DMS KEYWORD_WITH_ALT(150, 300, 150, 0,
yyextra->ARB_texture_multisample_enable, USAMPLER2DMS);
-sampler2DMSArray KEYWORD_WITH_ALT(150, 300, 150, 0,
yyextra->ARB_texture_multisample_enable, SAMPLER2DMSARRAY);
-isampler2DMSArray KEYWORD_WITH_ALT(150, 300, 150, 0,
yyextra->ARB_texture_multisample_enable, ISAMPLER2DMSARRAY);
-usampler2DMSArray KEYWORD_WITH_ALT(150, 300, 150, 0,
yyextra->ARB_texture_multisample_enable, USAMPLER2DMSARRAY);
+ /* these are needed for GLES 3.1 */
+sampler2DMSKEYWORD_WITH_ALT(150, 300, 150, 310,
yyextra->ARB_texture_multisample_enable, SAMPLER2DMS);
+isampler2DMS KEYWORD_WITH_ALT(150, 300, 150, 310,
yyextra->ARB_texture_multisample_enable, ISAMPLER2DMS);
+usampler2DMS KEYWORD_WITH_ALT(150, 300, 150, 310,
yyextra->ARB_texture_multisample_enable, USAMPLER2DMS);
+sampler2DMSArray KEYWORD_WITH_ALT(150, 300, 150, 310,
yyextra->ARB_texture_multisample_enable, SAMPLER2DMSARRAY);
+isampler2DMSArray KEYWORD_WITH_ALT(150, 300, 150, 310,
yyextra->ARB_texture_multisample_enable, ISAMPLER2DMSARRAY);
+usampler2DMSArray KEYWORD_WITH_ALT(150, 300, 150, 310,
yyextra->ARB_texture_multisample_enable, USAMPLER2DMSARRAY);
/* keywords available with ARB_texture_cube_map_array_enable extension on
desktop GLSL */
samplerCubeArray KEYWORD_WITH_ALT(400, 0, 400, 0,
yyextra->ARB_texture_cube_map_array_enable, SAMPLERCUBEARRAY);
diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
index 9a0c24e..a231d96 100644
--- a/src/glsl/glsl_parser_extras.h
+++ b/src/glsl/glsl_parser_extras.h
@@ -221,6 +221,11 @@ struct _mesa_glsl_parse_state {
|| EXT_separate_shader_objects_enable;
}
+ bool has_texture_multisample() const
+ {
+ return ARB_texture_multisample_enable || is_version(150, 310);
+ }
+
bool has_double() const
{
return ARB_gpu_shader_fp64_enable || is_version(400, 0);
--
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl/es31:Allow GL_ARB_TEXTURE_MULTISAMPLE in GLSL ES 3.10
Sorry about the last patch, I sent a V3 of below.
/Marta
From: Lofstedt, Marta
Sent: Monday, June 15, 2015 1:36 PM
To: Ilia Mirkin; Marta Lofstedt
Cc: mesa-dev@lists.freedesktop.org
Subject: RE: [Mesa-dev] [PATCH] glsl/es31:Allow GL_ARB_TEXTURE_MULTISAMPLE in
GLSL ES 3.10
> -Original Message-
> From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On
> Behalf Of Ilia Mirkin
> Sent: Tuesday, May 12, 2015 5:00 PM
> To: Marta Lofstedt
> Cc: mesa-dev@lists.freedesktop.org
> Subject: Re: [Mesa-dev] [PATCH] glsl/es31:Allow
> GL_ARB_TEXTURE_MULTISAMPLE in GLSL ES 3.10
>
> On Tue, May 12, 2015 at 10:53 AM, Marta Lofstedt
> wrote:
> > From: Marta Lofstedt
> >
> > Signed-off-by: Marta Lofstedt
> > ---
> > src/glsl/builtin_functions.cpp | 3 +--
> > src/glsl/builtin_types.cpp | 2 +-
> > src/glsl/glsl_lexer.ll | 13 +++--
> > src/glsl/glsl_parser_extras.h | 5 +
> > 4 files changed, 14 insertions(+), 9 deletions(-)
> >
> > diff --git a/src/glsl/builtin_functions.cpp
> > b/src/glsl/builtin_functions.cpp index 1df6956..76aff14 100644
> > --- a/src/glsl/builtin_functions.cpp
> > +++ b/src/glsl/builtin_functions.cpp
> > @@ -270,8 +270,7 @@ texture_array(const _mesa_glsl_parse_state
> *state)
> > static bool texture_multisample(const _mesa_glsl_parse_state *state)
> > {
> > - return state->is_version(150, 0) ||
> > - state->ARB_texture_multisample_enable;
> > + return state->has_texture_multisample();
> > }
> >
> > static bool
> > diff --git a/src/glsl/builtin_types.cpp b/src/glsl/builtin_types.cpp
> > index d92e2eb..9968f7c 100644
> > --- a/src/glsl/builtin_types.cpp
> > +++ b/src/glsl/builtin_types.cpp
> > @@ -307,7 +307,7 @@ _mesa_glsl_initialize_types(struct
> _mesa_glsl_parse_state *state)
> >add_type(symbols, glsl_type::usamplerCubeArray_type);
> > }
> >
> > - if (state->ARB_texture_multisample_enable) {
> > + if (state->has_texture_multisample()) {
>
> Oooh, nice catch! This was previously going to not auto-add the types for
> GLSL 1.50!
>
> >add_type(symbols, glsl_type::sampler2DMS_type);
> >add_type(symbols, glsl_type::isampler2DMS_type);
> >add_type(symbols, glsl_type::usampler2DMS_type); diff --git
> > a/src/glsl/glsl_lexer.ll b/src/glsl/glsl_lexer.ll index
> > 10db5b8..3597435 100644
> > --- a/src/glsl/glsl_lexer.ll
> > +++ b/src/glsl/glsl_lexer.ll
> > @@ -341,12 +341,13 @@ usampler2DArray KEYWORD(130, 300, 130,
> 300, USAMPLER2DARRAY);
> >
> > /* additional keywords in ARB_texture_multisample, included in GLSL
> 1.50 */
> > /* these are reserved but not defined in GLSL 3.00 */
> > -sampler2DMSKEYWORD_WITH_ALT(150, 300, 150, 0, yyextra-
> >ARB_texture_multisample_enable, SAMPLER2DMS);
> > -isampler2DMS KEYWORD_WITH_ALT(150, 300, 150, 0, yyextra-
> >ARB_texture_multisample_enable, ISAMPLER2DMS);
> > -usampler2DMS KEYWORD_WITH_ALT(150, 300, 150, 0, yyextra-
> >ARB_texture_multisample_enable, USAMPLER2DMS);
> > -sampler2DMSArray KEYWORD_WITH_ALT(150, 300, 150, 0, yyextra-
> >ARB_texture_multisample_enable, SAMPLER2DMSARRAY);
> > -isampler2DMSArray KEYWORD_WITH_ALT(150, 300, 150, 0,
> > yyextra->ARB_texture_multisample_enable, ISAMPLER2DMSARRAY);
> > -usampler2DMSArray KEYWORD_WITH_ALT(150, 300, 150, 0,
> > yyextra->ARB_texture_multisample_enable, USAMPLER2DMSARRAY);
> > + /* these are needed for GLES 3.1 */
> > +sampler2DMSKEYWORD_WITH_ALT(150, 300, 150, 310, yyextra-
> >ARB_texture_multisample_enable, SAMPLER2DMS);
> > +isampler2DMS KEYWORD_WITH_ALT(150, 300, 150, 310, yyextra-
> >ARB_texture_multisample_enable, ISAMPLER2DMS);
> > +usampler2DMS KEYWORD_WITH_ALT(150, 300, 150, 310, yyextra-
> >ARB_texture_multisample_enable, USAMPLER2DMS);
> > +sampler2DMSArray KEYWORD_WITH_ALT(150, 300, 150, 310, yyextra-
> >ARB_texture_multisample_enable, SAMPLER2DMSARRAY);
> > +isampler2DMSArray KEYWORD_WITH_ALT(150, 300, 150, 310,
> > +yyextra->ARB_texture_multisample_enable, ISAMPLER2DMSARRAY);
> > +usampler2DMSArray KEYWORD_WITH_ALT(150, 300, 150, 310,
> > +yyextra->ARB_texture_multisample_enable, USAMPLER2DMSARRAY);
> >
> > /* keywords available with ARB_texture_cube_map_array_enable
> extension on desktop GLSL */
> > samplerCubeArray KEYWORD_WITH_ALT(400, 0, 400, 0, yyextra-
> >ARB_texture_cube_map_array_enable, SAMPLERCUBEARRAY);
> > diff --git a/src/glsl/glsl_parser_extras.h
> > b/src/glsl/glsl_parser_extras.h index 4612071..ce78622 100644
> > --- a/src/glsl/glsl_parser_extras.h
> > +++ b/src/glsl/glsl_parser_extras.h
> > @@ -221,6 +221,11 @@ struct _mesa_glsl_parse_state {
> > || EXT_separate_shader_objects_enable;
> > }
> >
> > + bool has_texture_multisample() const
> > + {
> > + return ARB_texture_multisample_enable || is_version(410, 310);
>
> This should certainly be is_version(150, 310) right?
>
Yes, of course, fixed in V2.
/Marta
> > + }
> > +
> > bool has_double
Re: [Mesa-dev] [PATCH 03/14] mesa: Fix conditions to test signed, unsigned integer format
On Tue, 2015-06-23 at 08:54 +0200, Iago Toral wrote:
> On Mon, 2015-06-22 at 12:35 -0700, Anuj Phogat wrote:
> > On Sun, Jun 21, 2015 at 11:25 PM, Iago Toral wrote:
> > > On Fri, 2015-06-19 at 13:32 -0700, Anuj Phogat wrote:
> > >> On Thu, Jun 18, 2015 at 11:41 PM, Iago Toral wrote:
> > >> > On Thu, 2015-06-18 at 09:19 -0700, Anuj Phogat wrote:
> > >> >> On Thu, Jun 18, 2015 at 7:09 AM, Iago Toral wrote:
> > >> >> > On Tue, 2015-06-16 at 11:15 -0700, Anuj Phogat wrote:
> > >> >> >> Signed-off-by: Anuj Phogat
> > >> >> >> Cc:
> > >> >> >> ---
> > >> >> >> src/mesa/main/readpix.c | 2 ++
> > >> >> >> 1 file changed, 2 insertions(+)
> > >> >> >>
> > >> >> >> diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
> > >> >> >> index caa2648..a9416ef 100644
> > >> >> >> --- a/src/mesa/main/readpix.c
> > >> >> >> +++ b/src/mesa/main/readpix.c
> > >> >> >> @@ -160,10 +160,12 @@ _mesa_readpixels_needs_slow_path(const
> > >> >> >> struct gl_context *ctx, GLenum format,
> > >> >> >>srcType = _mesa_get_format_datatype(rb->Format);
> > >> >> >>
> > >> >> >>if ((srcType == GL_INT &&
> > >> >> >> + _mesa_is_enum_format_integer(format) &&
> > >> >> >> (type == GL_UNSIGNED_INT ||
> > >> >> >> type == GL_UNSIGNED_SHORT ||
> > >> >> >> type == GL_UNSIGNED_BYTE)) ||
> > >> >> >>(srcType == GL_UNSIGNED_INT &&
> > >> >> >> + _mesa_is_enum_format_integer(format) &&
> > >> >> >> (type == GL_INT ||
> > >> >> >> type == GL_SHORT ||
> > >> >> >> type == GL_BYTE))) {
> > >> >> >
> > >> >> > As far as I understand this code we are trying to see if we can use
> > >> >> > memcpy to directly copy the contents of the framebuffer to the
> > >> >> > destination buffer. In that case, as long as the src/dst types have
> > >> >> > different sign we can't just use memcpy, right? In fact it looks
> > >> >> > like we
> > >> >> > might need to expand the checks to include the cases where srcType
> > >> >> > is
> > >> >> > GL_(UNSIGNED_)SHORT and GL_(UNSIGNED_)BYTE as well.
> > >> >> >
> > >> >> srcType returned by _mesa_get_format_datatype() is one of:
> > >> >> GL_UNSIGNED_NORMALIZED
> > >> >> GL_SIGNED_NORMALIZED
> > >> >> GL_UNSIGNED_INT
> > >> >> GL_INT
> > >> >> GL_FLOAT
> > >> >> So, the suggested checks for srcType are not required.
> > >> >
> > >> > Oh, right, although I think that does not invalidate my point: can we
> > >> > memcpy from a GL_UNSIGNED_NORMALIZED to a format with type GL_FLOAT or
> > >> > GL_SIGNED_NORMALIZED? It does not look like these checks here are
> > >> > thorough.
> > >> >
> > >> Helper function _mesa_need_signed_unsigned_int_conversion() is
> > >> meant to do the checks only for integer formats. May be add another
> > >> function to do the missing checks for other formats?
> > >
> > > I have no concerns about the _mesa_need_signed_unsigned_int_conversion
> > > function that you add in a later patch for your PBO work, my concern is
> > > related to the fact that you are assuming that the checks that you need
> > > in the PBO path are the same that we have in
> > > _mesa_readpixels_needs_slow_path, so you make both the same when I think
> > > they are trying to address different things.
> > >
> > > In your PBO code, you can't handle signed/unsigned integer conversions,
> > > so you need to detect that and fall back to another path. That should be
> > > fine I guess and the function _mesa_need_signed_unsigned_int_conversion
> > > does what you need, so no problems there.
> > >
> > > However, in _mesa_readpixels_needs_slow_path I think we don't want to
> > > just do integer checking. The purpose of the function is to tell whether
> > > we can use memcpy to copy pixels from the framebuffer to the dst, and if
> > > we have types with different signs, *whether they are integer or not*,
> > > we can't, so limiting the check only to integer types does not look
> > > right to me. The key aspect here is that what this function needs to
> > > check is not specific to integer types, even if the current code only
> > > seems to check things when the framebuffer has an integer format.
> > >
> > >> > In any case, that's beyond the point of your patch. Talking
> > >> > specifically
> > >> > about your patch: can we memcpy, for example, from a _signed_ integer
> > >> > format like MESA_FORMAT_R_SINT8 to an _unsigned_ format (integer or
> > >> > not)? I don't think we can, in which case your patch would not look
> > >> > correct to me.
> > >> >
> > >> Reading integer format to a non integer format is not allowed in
> > >> glReadPixels. That's why those cases are not relevant here and
> > >> we just check for integer formats. From ext_texture_integer:
> > >> "INVALID_OPERATON is generated by ReadPixels if is
> > >> an integer format and the color buffer is not an integer format, or
> > >> if is not an integer format and the color buffer is an
> > >> integer format."
> > >
> > > Right, that
Re: [Mesa-dev] [PATCH 06/17] i965/blorp: Explicitly set execution sizes for new'd instructions
On Thu, Jun 18, 2015 at 05:51:35PM -0700, Jason Ekstrand wrote:
> This doesn't affect instructions allocated using the builder.
> ---
> src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
Reviewed-by: Topi Pohjolainen
>
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
> b/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
> index c1b7609..f655a0c 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
> @@ -72,7 +72,7 @@ brw_blorp_eu_emitter::emit_kill_if_outside_rect(const
> struct brw_reg &x,
> emit_cmp(BRW_CONDITIONAL_L, x, dst_x1)->predicate = BRW_PREDICATE_NORMAL;
> emit_cmp(BRW_CONDITIONAL_L, y, dst_y1)->predicate = BRW_PREDICATE_NORMAL;
>
> - fs_inst *inst = new (mem_ctx) fs_inst(BRW_OPCODE_AND, g1, f0, g1);
> + fs_inst *inst = new (mem_ctx) fs_inst(BRW_OPCODE_AND, 16, g1, f0, g1);
> inst->force_writemask_all = true;
> insts.push_tail(inst);
> }
> @@ -83,7 +83,7 @@ brw_blorp_eu_emitter::emit_texture_lookup(const struct
> brw_reg &dst,
>unsigned base_mrf,
>unsigned msg_length)
> {
> - fs_inst *inst = new (mem_ctx) fs_inst(op, dst, brw_message_reg(base_mrf),
> + fs_inst *inst = new (mem_ctx) fs_inst(op, 16, dst,
> brw_message_reg(base_mrf),
> fs_reg(0u));
>
> inst->base_mrf = base_mrf;
> @@ -118,7 +118,8 @@ brw_blorp_eu_emitter::emit_combine(enum opcode
> combine_opcode,
> {
> assert(combine_opcode == BRW_OPCODE_ADD || combine_opcode ==
> BRW_OPCODE_AVG);
>
> - insts.push_tail(new (mem_ctx) fs_inst(combine_opcode, dst, src_1, src_2));
> + insts.push_tail(new (mem_ctx) fs_inst(combine_opcode, 16, dst,
> + src_1, src_2));
> }
>
> fs_inst *
> @@ -126,7 +127,7 @@ brw_blorp_eu_emitter::emit_cmp(enum brw_conditional_mod
> op,
> const struct brw_reg &x,
> const struct brw_reg &y)
> {
> - fs_inst *cmp = new (mem_ctx) fs_inst(BRW_OPCODE_CMP,
> + fs_inst *cmp = new (mem_ctx) fs_inst(BRW_OPCODE_CMP, 16,
> vec16(brw_null_reg()), x, y);
> cmp->conditional_mod = op;
> insts.push_tail(cmp);
> --
> 2.4.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 07/17] i965/fs: Move offset() and half() to the fs_builder
On Thu, Jun 18, 2015 at 05:51:36PM -0700, Jason Ekstrand wrote:
> We want to move these into the builder so that they know the current
> builder's dispatch width. This will be needed by a later commit.
> ---
> src/mesa/drivers/dri/i965/brw_fs.cpp | 52 ++
> src/mesa/drivers/dri/i965/brw_fs_builder.h | 46 +
> src/mesa/drivers/dri/i965/brw_fs_cse.cpp | 2 +-
> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 60 +--
> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 149
> ++-
> src/mesa/drivers/dri/i965/brw_ir_fs.h| 51 -
> 6 files changed, 182 insertions(+), 178 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 4f98d63..c13ac7d 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -267,7 +267,7 @@ fs_visitor::VARYING_PULL_CONSTANT_LOAD(const fs_builder
> &bld,
> inst->mlen = 1 + dispatch_width / 8;
> }
>
> - bld.MOV(dst, offset(vec4_result, (const_offset & 3) * scale));
> + bld.MOV(dst, bld.offset(vec4_result, (const_offset & 3) * scale));
> }
>
> /**
> @@ -361,7 +361,12 @@ fs_inst::is_copy_payload(const brw::simple_allocator
> &grf_alloc) const
>reg.width = this->src[i].width;
>if (!this->src[i].equals(reg))
> return false;
> - reg = ::offset(reg, 1);
> +
> + if (i < this->header_size) {
> + reg.reg_offset += 1;
> + } else {
> + reg.reg_offset += this->exec_size / 8;
> + }
The latter branch is new functionality, isn't it? There is no consideration
for header_size in the offset() utility.
> }
>
> return true;
> @@ -963,7 +968,7 @@ fs_visitor::emit_fragcoord_interpolation(bool
> pixel_center_integer,
> } else {
>bld.ADD(wpos, this->pixel_x, fs_reg(0.5f));
> }
> - wpos = offset(wpos, 1);
> + wpos = bld.offset(wpos, 1);
>
> /* gl_FragCoord.y */
> if (!flip && pixel_center_integer) {
> @@ -979,7 +984,7 @@ fs_visitor::emit_fragcoord_interpolation(bool
> pixel_center_integer,
>
>bld.ADD(wpos, pixel_y, fs_reg(offset));
> }
> - wpos = offset(wpos, 1);
> + wpos = bld.offset(wpos, 1);
>
> /* gl_FragCoord.z */
> if (devinfo->gen >= 6) {
> @@ -989,7 +994,7 @@ fs_visitor::emit_fragcoord_interpolation(bool
> pixel_center_integer,
> this->delta_xy[BRW_WM_PERSPECTIVE_PIXEL_BARYCENTRIC],
> interp_reg(VARYING_SLOT_POS, 2));
> }
> - wpos = offset(wpos, 1);
> + wpos = bld.offset(wpos, 1);
>
> /* gl_FragCoord.w: Already set up in emit_interpolation */
> bld.MOV(wpos, this->wpos_w);
> @@ -1072,7 +1077,7 @@ fs_visitor::emit_general_interpolation(fs_reg attr,
> const char *name,
> /* If there's no incoming setup data for this slot, don't
>* emit interpolation for it.
>*/
> - attr = offset(attr, type->vector_elements);
> + attr = bld.offset(attr, type->vector_elements);
> location++;
> continue;
>}
> @@ -1087,7 +1092,7 @@ fs_visitor::emit_general_interpolation(fs_reg attr,
> const char *name,
> interp = suboffset(interp, 3);
> interp.type = attr.type;
> bld.emit(FS_OPCODE_CINTERP, attr, fs_reg(interp));
> -attr = offset(attr, 1);
> +attr = bld.offset(attr, 1);
> }
>} else {
> /* Smooth/noperspective interpolation case. */
> @@ -1125,7 +1130,7 @@ fs_visitor::emit_general_interpolation(fs_reg attr,
> const char *name,
> if (devinfo->gen < 6 && interpolation_mode ==
> INTERP_QUALIFIER_SMOOTH) {
>bld.MUL(attr, attr, this->pixel_w);
> }
> -attr = offset(attr, 1);
> +attr = bld.offset(attr, 1);
> }
>
>}
> @@ -1227,19 +1232,19 @@ fs_visitor::emit_samplepos_setup()
> if (dispatch_width == 8) {
>abld.MOV(int_sample_x, fs_reg(sample_pos_reg));
> } else {
> - abld.half(0).MOV(half(int_sample_x, 0), fs_reg(sample_pos_reg));
> - abld.half(1).MOV(half(int_sample_x, 1),
> + abld.half(0).MOV(abld.half(int_sample_x, 0), fs_reg(sample_pos_reg));
> + abld.half(1).MOV(abld.half(int_sample_x, 1),
> fs_reg(suboffset(sample_pos_reg, 16)));
> }
> /* Compute gl_SamplePosition.x */
> compute_sample_position(pos, int_sample_x);
> - pos = offset(pos, 1);
> + pos = abld.offset(pos, 1);
> if (dispatch_width == 8) {
>abld.MOV(int_sample_y, fs_reg(suboffset(sample_pos_reg, 1)));
> } else {
> - abld.half(0).MOV(half(int_sample_y, 0),
> + abld.half(0).MOV(abld.half(int_sample_y, 0),
> fs_reg(suboffset(sample_pos_reg, 1)));
> - abld.half(1).MOV(half(int_sample_y, 1),
> + abld.half(1).MOV(abld.half(int_sample_y, 1),
> fs_reg(suboffset(sample_po
Re: [Mesa-dev] [PATCH 10/17] i965/fs: Use exec_size for determining regs read/written and partial writes
On Thu, Jun 18, 2015 at 05:51:39PM -0700, Jason Ekstrand wrote:
> ---
> src/mesa/drivers/dri/i965/brw_fs.cpp | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Topi Pohjolainen
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 61235d7..cff27e7 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -101,7 +101,7 @@ fs_inst::init(enum opcode opcode, uint8_t exec_size,
> const fs_reg &dst,
> case MRF:
> case ATTR:
>this->regs_written =
> - DIV_ROUND_UP(MAX2(dst.width * dst.stride, 1) * type_sz(dst.type),
> 32);
> + DIV_ROUND_UP(MAX2(exec_size * dst.stride, 1) * type_sz(dst.type),
> 32);
>break;
> case BAD_FILE:
>this->regs_written = 0;
> @@ -718,7 +718,7 @@ bool
> fs_inst::is_partial_write() const
> {
> return ((this->predicate && this->opcode != BRW_OPCODE_SEL) ||
> - (this->dst.width * type_sz(this->dst.type)) < 32 ||
> + (this->exec_size * type_sz(this->dst.type)) < 32 ||
> !this->dst.is_contiguous());
> }
>
> @@ -772,8 +772,8 @@ fs_inst::regs_read(int arg) const
>if (src[arg].stride == 0) {
> return 1;
>} else {
> - int size = src[arg].width * src[arg].stride *
> type_sz(src[arg].type);
> - return (size + 31) / 32;
> + int size = this->exec_size * src[arg].stride *
> type_sz(src[arg].type);
> + return DIV_ROUND_UP(size, 32);
>}
> case MRF:
>unreachable("MRF registers are not allowed as sources");
> --
> 2.4.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 14/17] i965/fs_builder: Use dispatch_width instead of reg.width for offset and half
On Thu, Jun 18, 2015 at 05:51:43PM -0700, Jason Ekstrand wrote:
> ---
> src/mesa/drivers/dri/i965/brw_fs_builder.h | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_builder.h
> b/src/mesa/drivers/dri/i965/brw_fs_builder.h
> index 7d3c8ab..58519d7 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_builder.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs_builder.h
> @@ -161,7 +161,7 @@ namespace brw {
> case MRF:
> case ATTR:
How would you feel about an assertion here:
assert(reg.stride);
> return byte_offset(reg,
> - delta * MAX2(reg.width * reg.stride, 1) *
> + delta * dispatch_width() * reg.stride *
> type_sz(reg.type));
> case UNIFORM:
> reg.reg_offset += delta;
> @@ -185,9 +185,9 @@ namespace brw {
>
> case GRF:
> case MRF:
> -assert(reg.width == 16);
> -reg.width = 8;
> -return horiz_offset(reg, 8 * idx);
> +assert(dispatch_width() == 16);
> +reg.width = dispatch_width() / 2;
> +return horiz_offset(reg, (dispatch_width() / 2) * idx);
>
> case ATTR:
> case HW_REG:
> --
> 2.4.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 08/17] i965/fs: Make better use of the builder in shader_time
On Thu, Jun 18, 2015 at 05:51:37PM -0700, Jason Ekstrand wrote: > Previously, we were just depending on register widths to ensure that > various things were exec_size of 1 etc. Now, we do so explicitly using the > builder. > --- > src/mesa/drivers/dri/i965/brw_fs.cpp | 14 -- > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp > b/src/mesa/drivers/dri/i965/brw_fs.cpp > index c13ac7d..740b51d 100644 > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp > @@ -557,7 +557,7 @@ fs_visitor::get_timestamp(const fs_builder &bld) > /* We want to read the 3 fields we care about even if it's not enabled in > * the dispatch. > */ > - bld.exec_all().MOV(dst, ts); > + bld.group(4, 0).exec_all().MOV(dst, ts); Just to make sure I understand correctly, we want SIMD4 in order to read wide enough to get all the mentioned 3 fields? > > /* The caller wants the low 32 bits of the timestamp. Since it's running > * at the GPU clock rate of ~1.2ghz, it will roll over every ~3 seconds, > @@ -637,17 +637,19 @@ fs_visitor::emit_shader_time_end() > start.negate = true; > fs_reg diff = fs_reg(GRF, alloc.allocate(1), BRW_REGISTER_TYPE_UD, 1); > diff.set_smear(0); > - ibld.ADD(diff, start, shader_end_time); > + > + const fs_builder cbld = ibld.group(1, 0); > + cbld.group(1, 0).ADD(diff, start, shader_end_time); > > /* If there were no instructions between the two timestamp gets, the diff > * is 2 cycles. Remove that overhead, so I can forget about that when > * trying to determine the time taken for single instructions. > */ > - ibld.ADD(diff, diff, fs_reg(-2u)); > - SHADER_TIME_ADD(ibld, type, diff); > - SHADER_TIME_ADD(ibld, written_type, fs_reg(1u)); > + cbld.ADD(diff, diff, fs_reg(-2u)); > + SHADER_TIME_ADD(cbld, type, diff); > + SHADER_TIME_ADD(cbld, written_type, fs_reg(1u)); > ibld.emit(BRW_OPCODE_ELSE); > - SHADER_TIME_ADD(ibld, reset_type, fs_reg(1u)); > + SHADER_TIME_ADD(cbld, reset_type, fs_reg(1u)); > ibld.emit(BRW_OPCODE_ENDIF); > } > > -- > 2.4.3 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 91034] New account request
https://bugs.freedesktop.org/show_bug.cgi?id=91034 Daniel Stone changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #2 from Daniel Stone --- ah yes, graphics gets us all eventually - welcome! -- 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 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 17/17] i965/fs: Remove the width field from fs_reg
On Thu, Jun 18, 2015 at 05:51:46PM -0700, Jason Ekstrand wrote: > As of now, the width field is no longer used for anything. The width field > "seemed like a good idea at the time" but is actually entirely redundant > with the instruction's execution size. Initially, it gave us the ability > to easily set the instructions execution size based entirely on register > widths. With the builder, we can easiliy set the sizes explicitly and the > width field doesn't have as much purpose. At this point, it's just > redundant information that can get out of sync so it really needs to go. > --- > src/mesa/drivers/dri/i965/brw_fs.cpp | 62 > -- > src/mesa/drivers/dri/i965/brw_fs_builder.h | 21 ++-- > .../drivers/dri/i965/brw_fs_copy_propagation.cpp | 4 -- > src/mesa/drivers/dri/i965/brw_fs_cse.cpp | 6 +-- > src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp | 4 +- > .../drivers/dri/i965/brw_fs_register_coalesce.cpp | 1 - > src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 26 - > src/mesa/drivers/dri/i965/brw_ir_fs.h | 13 + > 8 files changed, 30 insertions(+), 107 deletions(-) > I started tagging one by one but apart from patch seven where I have doubts that I understand the copy-payload related logic well enough all the rest is: Reviewed-by: Topi Pohjolainen ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] mesa: remove unnecessary checks in _mesa_readpixels_needs_slow_path
readpixels_can_use_memcpy will later call _mesa_format_matches_format_and_type
which does much tighter checks than these to decide if we can use
memcpy for readpixels.
Also, the checks do not seem to be extensive enough anyway, since we are
checking for signed/unsigned conversion only when the framebuffer has integers,
but the same checks could be done for other types anyway, since as long as
there is a signed/unsigned conversion we can't memcpy.
No regressions observed on i965/llvmpipe.
---
The way gallium uses _mesa_format_matches_format_and_type and
_mesa_readpixels_needs_slow_path is a bit different, they call these
functions to decide if they want to fallback to Mesa's implementation
of ReadPixels, not exactly to check if we can memcpy... so it is unclear
to me if some combinations may still need these checks to make the right
decision. I did not see any regressions with llvmpipe at least, so I am
assuming that they are not needed, but maybe someone wants to give this
patch a test run on nouveau or radeon, just in case. If they are needed
I guess the right thing would be to move the checks to st_cb_readpixels.c.
src/mesa/main/readpix.c | 16
1 file changed, 16 deletions(-)
diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
index a3357cd..e256695 100644
--- a/src/mesa/main/readpix.c
+++ b/src/mesa/main/readpix.c
@@ -128,7 +128,6 @@ _mesa_readpixels_needs_slow_path(const struct gl_context
*ctx, GLenum format,
{
struct gl_renderbuffer *rb =
_mesa_get_read_renderbuffer_for_format(ctx, format);
- GLenum srcType;
assert(rb);
@@ -153,21 +152,6 @@ _mesa_readpixels_needs_slow_path(const struct gl_context
*ctx, GLenum format,
return GL_TRUE;
}
- /* Conversion between signed and unsigned integers needs masking
- * (it isn't just memcpy). */
- srcType = _mesa_get_format_datatype(rb->Format);
-
- if ((srcType == GL_INT &&
- (type == GL_UNSIGNED_INT ||
-type == GL_UNSIGNED_SHORT ||
-type == GL_UNSIGNED_BYTE)) ||
- (srcType == GL_UNSIGNED_INT &&
- (type == GL_INT ||
-type == GL_SHORT ||
-type == GL_BYTE))) {
- return GL_TRUE;
- }
-
/* And finally, see if there are any transfer ops. */
return get_readpixels_transfer_ops(ctx, rb->Format, format, type,
uses_blit) != 0;
--
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] mesa : NULL check InfoLog
From: Marta Lofstedt When a program is compiled, but linking failed the sh->InfoLog could be NULL. This is expoloited by OpenGL ES 3.1 conformance tests. Signed-off-by: Marta Lofstedt --- src/mesa/main/shaderapi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c index a4296ad..c783c69 100644 --- a/src/mesa/main/shaderapi.c +++ b/src/mesa/main/shaderapi.c @@ -1920,8 +1920,8 @@ _mesa_create_shader_program(struct gl_context* ctx, GLboolean separate, } #endif } - -ralloc_strcat(&shProg->InfoLog, sh->InfoLog); + if (sh->InfoLog) + ralloc_strcat(&shProg->InfoLog, sh->InfoLog); } delete_shader(ctx, shader); -- 1.9.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] glsl: use consistent version string format
---
src/glsl/glsl_parser_extras.h | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
index 9a0c24e..02ddbbd 100644
--- a/src/glsl/glsl_parser_extras.h
+++ b/src/glsl/glsl_parser_extras.h
@@ -129,7 +129,7 @@ struct _mesa_glsl_parse_state {
bool check_explicit_attrib_stream_allowed(YYLTYPE *locp)
{
if (!this->has_explicit_attrib_stream()) {
- const char *const requirement = "GL_ARB_gpu_shader5 extension or GLSL
400";
+ const char *const requirement = "GL_ARB_gpu_shader5 extension or GLSL
4.00";
_mesa_glsl_error(locp, this, "explicit stream requires %s",
requirement);
@@ -144,8 +144,8 @@ struct _mesa_glsl_parse_state {
{
if (!this->has_explicit_attrib_location()) {
const char *const requirement = this->es_shader
-? "GLSL ES 300"
-: "GL_ARB_explicit_attrib_location extension or GLSL 330";
+? "GLSL ES 3.00"
+: "GL_ARB_explicit_attrib_location extension or GLSL 3.30";
_mesa_glsl_error(locp, this, "%s explicit location requires %s",
mode_string(var), requirement);
@@ -160,8 +160,8 @@ struct _mesa_glsl_parse_state {
{
if (!this->has_separate_shader_objects()) {
const char *const requirement = this->es_shader
-? "GL_EXT_separate_shader_objects extension or GLSL ES 310"
-: "GL_ARB_separate_shader_objects extension or GLSL 420";
+? "GL_EXT_separate_shader_objects extension or GLSL ES 3.10"
+: "GL_ARB_separate_shader_objects extension or GLSL 4.20";
_mesa_glsl_error(locp, this, "%s explicit location requires %s",
mode_string(var), requirement);
@@ -177,9 +177,9 @@ struct _mesa_glsl_parse_state {
if (!this->has_explicit_attrib_location() ||
!this->has_explicit_uniform_location()) {
const char *const requirement = this->es_shader
-? "GLSL ES 310"
+? "GLSL ES 3.10"
: "GL_ARB_explicit_uniform_location and either "
- "GL_ARB_explicit_attrib_location or GLSL 330.";
+ "GL_ARB_explicit_attrib_location or GLSL 3.30.";
_mesa_glsl_error(locp, this,
"uniform explicit location requires %s",
--
2.4.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] freedreno: use consistent version string format
---
src/gallium/drivers/freedreno/freedreno_screen.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c
b/src/gallium/drivers/freedreno/freedreno_screen.c
index b3b5462..7330cd9 100644
--- a/src/gallium/drivers/freedreno/freedreno_screen.c
+++ b/src/gallium/drivers/freedreno/freedreno_screen.c
@@ -68,7 +68,7 @@ static const struct debug_named_value debug_options[] = {
{"fraghalf", FD_DBG_FRAGHALF, "Use half-precision in fragment
shader"},
{"nobin", FD_DBG_NOBIN, "Disable hw binning"},
{"optmsgs", FD_DBG_OPTMSGS,"Enable optimizer debug messages"},
- {"glsl120", FD_DBG_GLSL120,"Temporary flag to force GLSL 120
(rather than 130) on a3xx+"},
+ {"glsl120", FD_DBG_GLSL120,"Temporary flag to force GLSL 1.20
(rather than 1.30) on a3xx+"},
DEBUG_NAMED_VALUE_END
};
--
2.4.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl/es3.1: Fix up GL_ARB_compute_shader for GLSL ES 3.1
From: Marta Lofstedt
GL_ARB_compute_shader is limited for GLSL version 430.
This enables for GLSL ES version 310.
Signed-off-by: Marta Lofstedt
---
src/glsl/builtin_variables.cpp | 2 +-
src/glsl/glsl_parser.yy| 3 +--
src/glsl/glsl_parser_extras.h | 5 +
3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/glsl/builtin_variables.cpp b/src/glsl/builtin_variables.cpp
index a765d35..2681981 100644
--- a/src/glsl/builtin_variables.cpp
+++ b/src/glsl/builtin_variables.cpp
@@ -695,7 +695,7 @@ builtin_variable_generator::generate_constants()
}
}
- if (state->is_version(430, 0) || state->ARB_compute_shader_enable) {
+ if (state->has_compute_shader()) {
add_const("gl_MaxComputeAtomicCounterBuffers",
MAX_COMPUTE_ATOMIC_COUNTER_BUFFERS);
add_const("gl_MaxComputeAtomicCounters", MAX_COMPUTE_ATOMIC_COUNTERS);
add_const("gl_MaxComputeImageUniforms", MAX_COMPUTE_IMAGE_UNIFORMS);
diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
index 3ce9e10..757e548 100644
--- a/src/glsl/glsl_parser.yy
+++ b/src/glsl/glsl_parser.yy
@@ -1483,8 +1483,7 @@ layout_qualifier_id:
"invalid %s of %d specified",
local_size_qualifiers[i], $3);
YYERROR;
-} else if (!state->is_version(430, 0) &&
- !state->ARB_compute_shader_enable) {
+} else if (!state->has_compute_shader()) {
_mesa_glsl_error(& @3, state,
"%s qualifier requires GLSL 4.30 or "
"ARB_compute_shader",
diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
index a231d96..74838d3 100644
--- a/src/glsl/glsl_parser_extras.h
+++ b/src/glsl/glsl_parser_extras.h
@@ -231,6 +231,11 @@ struct _mesa_glsl_parse_state {
return ARB_gpu_shader_fp64_enable || is_version(400, 0);
}
+ bool has_compute_shader() const
+ {
+ return ARB_compute_shader_enable || is_version(430, 310);
+ }
+
void process_version_directive(YYLTYPE *locp, int version,
const char *ident);
--
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] mesa/es3.1: Enable GL_ARB_separate_shader_objects for GLES 3.1
From: Marta Lofstedt
Signed-off-by: Marta Lofstedt
---
src/mesa/main/get_hash_params.py | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/mesa/main/get_hash_params.py b/src/mesa/main/get_hash_params.py
index 74ff3ba..f7134a9 100644
--- a/src/mesa/main/get_hash_params.py
+++ b/src/mesa/main/get_hash_params.py
@@ -407,6 +407,10 @@ descriptor=[
{ "apis": ["GL", "GL_CORE", "GLES3"], "params": [
# GL_ARB_sampler_objects / GL 3.3 / GLES 3.0
[ "SAMPLER_BINDING", "LOC_CUSTOM, TYPE_INT, GL_SAMPLER_BINDING, NO_EXTRA" ],
+
+# GL_ARB_vertex_attrib_binding / GLES 3.1
+ [ "MAX_VERTEX_ATTRIB_RELATIVE_OFFSET",
"CONTEXT_ENUM(Const.MaxVertexAttribRelativeOffset), NO_EXTRA" ],
+ [ "MAX_VERTEX_ATTRIB_BINDINGS",
"CONTEXT_ENUM(Const.MaxVertexAttribBindings), NO_EXTRA" ],
]},
# Enums in OpenGL Core profile and ES 3.1
@@ -776,10 +780,6 @@ descriptor=[
[ "MAX_COMBINED_ATOMIC_COUNTER_BUFFERS",
"CONTEXT_INT(Const.MaxCombinedAtomicBuffers), extra_ARB_shader_atomic_counters"
],
[ "MAX_COMBINED_ATOMIC_COUNTERS",
"CONTEXT_INT(Const.MaxCombinedAtomicCounters),
extra_ARB_shader_atomic_counters" ],
-# GL_ARB_vertex_attrib_binding
- [ "MAX_VERTEX_ATTRIB_RELATIVE_OFFSET",
"CONTEXT_ENUM(Const.MaxVertexAttribRelativeOffset), NO_EXTRA" ],
- [ "MAX_VERTEX_ATTRIB_BINDINGS",
"CONTEXT_ENUM(Const.MaxVertexAttribBindings), NO_EXTRA" ],
-
# GL_ARB_shader_image_load_store
[ "MAX_IMAGE_UNITS", "CONTEXT_INT(Const.MaxImageUnits),
extra_ARB_shader_image_load_store"],
[ "MAX_COMBINED_IMAGE_UNITS_AND_FRAGMENT_OUTPUTS",
"CONTEXT_INT(Const.MaxCombinedImageUnitsAndFragmentOutputs),
extra_ARB_shader_image_load_store"],
--
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 06/17] i965/blorp: Explicitly set execution sizes for new'd instructions
Reviewed-by: Iago Toral Quiroga
On Thu, 2015-06-18 at 17:51 -0700, Jason Ekstrand wrote:
> This doesn't affect instructions allocated using the builder.
> ---
> src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
> b/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
> index c1b7609..f655a0c 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
> @@ -72,7 +72,7 @@ brw_blorp_eu_emitter::emit_kill_if_outside_rect(const
> struct brw_reg &x,
> emit_cmp(BRW_CONDITIONAL_L, x, dst_x1)->predicate = BRW_PREDICATE_NORMAL;
> emit_cmp(BRW_CONDITIONAL_L, y, dst_y1)->predicate = BRW_PREDICATE_NORMAL;
>
> - fs_inst *inst = new (mem_ctx) fs_inst(BRW_OPCODE_AND, g1, f0, g1);
> + fs_inst *inst = new (mem_ctx) fs_inst(BRW_OPCODE_AND, 16, g1, f0, g1);
> inst->force_writemask_all = true;
> insts.push_tail(inst);
> }
> @@ -83,7 +83,7 @@ brw_blorp_eu_emitter::emit_texture_lookup(const struct
> brw_reg &dst,
>unsigned base_mrf,
>unsigned msg_length)
> {
> - fs_inst *inst = new (mem_ctx) fs_inst(op, dst, brw_message_reg(base_mrf),
> + fs_inst *inst = new (mem_ctx) fs_inst(op, 16, dst,
> brw_message_reg(base_mrf),
> fs_reg(0u));
>
> inst->base_mrf = base_mrf;
> @@ -118,7 +118,8 @@ brw_blorp_eu_emitter::emit_combine(enum opcode
> combine_opcode,
> {
> assert(combine_opcode == BRW_OPCODE_ADD || combine_opcode ==
> BRW_OPCODE_AVG);
>
> - insts.push_tail(new (mem_ctx) fs_inst(combine_opcode, dst, src_1, src_2));
> + insts.push_tail(new (mem_ctx) fs_inst(combine_opcode, 16, dst,
> + src_1, src_2));
> }
>
> fs_inst *
> @@ -126,7 +127,7 @@ brw_blorp_eu_emitter::emit_cmp(enum brw_conditional_mod
> op,
> const struct brw_reg &x,
> const struct brw_reg &y)
> {
> - fs_inst *cmp = new (mem_ctx) fs_inst(BRW_OPCODE_CMP,
> + fs_inst *cmp = new (mem_ctx) fs_inst(BRW_OPCODE_CMP, 16,
> vec16(brw_null_reg()), x, y);
> cmp->conditional_mod = op;
> insts.push_tail(cmp);
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 05/17] i965/fs: Set the builder group for emitting FB-write stencil/AA alpha
Reviewed-by: Iago Toral Quiroga
On Thu, 2015-06-18 at 17:50 -0700, Jason Ekstrand wrote:
> ---
> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index b00825e..8a43ec8 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -1528,7 +1528,7 @@ fs_visitor::emit_single_fb_write(const fs_builder &bld,
>
> if (payload.aa_dest_stencil_reg) {
>sources[length] = fs_reg(GRF, alloc.allocate(1));
> - bld.exec_all().annotate("FB write stencil/AA alpha")
> + bld.group(8, 0).exec_all().annotate("FB write stencil/AA alpha")
> .MOV(sources[length],
>fs_reg(brw_vec8_grf(payload.aa_dest_stencil_reg, 0)));
>length++;
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/6] mesa/es3.1: Do not allow zero size multisampled textures
From: Marta Lofstedt According to GLES 3.1 CTS test: ES31-CTS.texture_storage_multisample. APIGLTexStorage2DMultisample. multisample_texture_tex_storage_2d_ invalid_and_border_case_texture_sizes. Textures of size 0x0 are not allowed for GL_TEXTURE_2D_MULTISAMPLE. Signed-off-by: Marta Lofstedt --- src/mesa/main/teximage.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c index 3d85615..c76ad54 100644 --- a/src/mesa/main/teximage.c +++ b/src/mesa/main/teximage.c @@ -1483,6 +1483,13 @@ _mesa_legal_texture_dimensions(struct gl_context *ctx, GLenum target, if (height > 0 && !_mesa_is_pow_two(height - 2 * border)) return GL_FALSE; } + /* + * according to GLES 3.1 CTS it is not OK with + * zero size multisampled textures + */ + if (width == 0 && height == 0 && GL_TEXTURE_2D_MULTISAMPLE == target) + return GL_FALSE; + return GL_TRUE; case GL_TEXTURE_3D: -- 1.9.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 0/6] OpenGL ES 3.1 API checks and corner cases.
This is a series of patches that solves a couple of API check and corner cases issues that the OpenGL ES 3.1 CTS exploits. Marta Lofstedt (6): mesa/es3.1: Do not allow zero size multisampled textures mesa/es3.1: Correct error code for illegal internal formats mesa/es3.1 : Correct error code for zero samples mesa/es3.1 : Correct error code for defect texture target mesa/es31: AtomicBufferBindings should be initialized to zero. mesa/es3.1: Fix error code for glCreateShaderProgram src/mesa/main/bufferobj.c | 11 --- src/mesa/main/shaderapi.c | 9 + src/mesa/main/teximage.c | 25 + src/mesa/main/texobj.c| 11 +++ 4 files changed, 53 insertions(+), 3 deletions(-) -- 1.9.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/6] mesa/es3.1: Correct error code for illegal internal formats
From: Marta Lofstedt
According to GLES 3.1 CTS test:
ES31-CTS.texture_storage_multisample.
APIGLTexStorage2DMultisample.
multisample_texture_tex_storage_2d_non_color_depth_or_stencil_
internal_formats_receted.
An illegal internal format should generate a
GL_INVALID_ENUM error.
Signed-off-by: Marta Lofstedt
---
src/mesa/main/teximage.c | 8
1 file changed, 8 insertions(+)
diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
index c76ad54..14af9cd 100644
--- a/src/mesa/main/teximage.c
+++ b/src/mesa/main/teximage.c
@@ -5617,6 +5617,14 @@ _mesa_texture_image_multisample(struct gl_context *ctx,
GLuint dims,
}
if (!is_renderable_texture_format(ctx, internalformat)) {
+ /* Accroding to OpenGL ES CTS, internal format error,
+ * should generate GL_INVALID_ENUM.
+ */
+ if(_mesa_is_gles31(ctx))
+ _mesa_error(ctx, GL_INVALID_ENUM,
+"%s(internalformat=%s)",
+func, _mesa_lookup_enum_by_nr(internalformat));
+ else
_mesa_error(ctx, GL_INVALID_OPERATION,
"%s(internalformat=%s)",
func, _mesa_lookup_enum_by_nr(internalformat));
--
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/6] mesa/es3.1 : Correct error code for zero samples
From: Marta Lofstedt
According to GLES 3.1 CTS test:
ES31-CTS.texture_storage_multisample.
APIGLTexStorage2DMultisample.
multisample_texture_tex_storage_2d_zero_sample-
A call to glTexStorageMultisample with target
GL_TEXTURE_2D_MULTISAMPLE and zero samples,
should return GL_INVALID_VALUE.
However, with above the piglit test:
gl-3.2-layered-rendering-framebuffertexture fails.
Hence, only limit this for GLES 3.1 contexts.
Signed-off-by: Marta Lofstedt
---
src/mesa/main/teximage.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
index 14af9cd..98f0223 100644
--- a/src/mesa/main/teximage.c
+++ b/src/mesa/main/teximage.c
@@ -5588,6 +5588,16 @@ _mesa_texture_image_multisample(struct gl_context *ctx,
GLuint dims,
GLenum sample_count_error;
bool dsa = strstr(func, "ture") ? true : false;
+ /*
+* According to OpenGL ES 3.1 CTS zero samples
+* should generate GL_INVALID_VALUE
+*/
+ if(samples == 0 && _mesa_is_gles31(ctx))
+ {
+ _mesa_error(ctx, GL_INVALID_VALUE, "%s(target)", func);
+ return;
+ }
+
if (!(ctx->Extensions.ARB_texture_multisample
&& _mesa_is_desktop_gl(ctx))) {
_mesa_error(ctx, GL_INVALID_OPERATION, "%s(unsupported)", func);
--
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 6/6] mesa/es3.1: Fix error code for glCreateShaderProgram
From: Marta Lofstedt
According to the OpenGL ES standard, 7.3.
For a call to glCreateShaderProgram with count < 0,
a GL_INVALID_VALUE error should be generated.
Signed-off-by: Marta Lofstedt
---
src/mesa/main/shaderapi.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index c783c69..266064d 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -1890,6 +1890,15 @@ _mesa_create_shader_program(struct gl_context* ctx,
GLboolean separate,
const GLuint shader = create_shader(ctx, type);
GLuint program = 0;
+ /*
+* According to OpenGL ES 3.1 standard 7.3: GL_INVALID_VALUE
+* should be generated, if count < 0.
+*/
+ if (_mesa_is_gles31(ctx) && count < 0) {
+ _mesa_error(ctx, GL_INVALID_VALUE, "glCreateShaderProgram (count < 0)");
+ return program;
+ }
+
if (shader) {
_mesa_ShaderSource(shader, count, strings, NULL);
--
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/6] mesa/es3.1 : Correct error code for defect texture target
From: Marta Lofstedt
According to GLES 3.1 CTS test:
ES31-CTS.texture_storage_multisample.
APIGLGetTexLevelParameterifv.
invalid_texture_target_rejected:
GL_INVALID_ENUM should be generated when
glGetTexLevelParameteriv is called with a defect
texture target.
Signed-off-by: Marta Lofstedt
---
src/mesa/main/texobj.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/src/mesa/main/texobj.c b/src/mesa/main/texobj.c
index c563f1e..c239deb 100644
--- a/src/mesa/main/texobj.c
+++ b/src/mesa/main/texobj.c
@@ -222,6 +222,17 @@ _mesa_get_current_tex_object(struct gl_context *ctx,
GLenum target)
return ctx->Extensions.ARB_texture_multisample
? ctx->Texture.ProxyTex[TEXTURE_2D_MULTISAMPLE_ARRAY_INDEX] : NULL;
default:
+ if(_mesa_is_gles31(ctx))
+ {
+/*
+ * According to OpenGL ES 3.1 CTS:
+ *
ES31-CTS.texture_storage_multisample.APIGLGetTexLevelParameterifv.
+ * invalid_value_argument_rejected
+ *
es31cTextureStorageMultisampleGetTexLevelParameterifvTests.cpp:1277
+ * INVALID_ENUM should be reported for bad targets.
+ */
+_mesa_error(ctx, GL_INVALID_ENUM, "%s(target)", __func__);
+ }
_mesa_problem(NULL, "bad target in _mesa_get_current_tex_object()");
return NULL;
}
--
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 5/6] mesa/es31: AtomicBufferBindings should be initialized to zero.
From: Marta Lofstedt
Accoring to GLES 3.1 CTS:
GLES 3.1 CTS: ES31-CTS.shader_atomic_counters.
basic-buffer-bind.
AtomicBufferBindings size and start should be initialized
to zero.
Signed-off-by: Marta Lofstedt
---
src/mesa/main/bufferobj.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
index 66dee68..94629b3 100644
--- a/src/mesa/main/bufferobj.c
+++ b/src/mesa/main/bufferobj.c
@@ -849,9 +849,14 @@ _mesa_init_buffer_objects( struct gl_context *ctx )
_mesa_reference_buffer_object(ctx,
&ctx->AtomicBufferBindings[i].BufferObject,
ctx->Shared->NullBufferObj);
- ctx->AtomicBufferBindings[i].Offset = -1;
- ctx->AtomicBufferBindings[i].Size = -1;
- }
+ if (_mesa_is_gles31(ctx)) {
+ ctx->AtomicBufferBindings[i].Offset = 0;
+ ctx->AtomicBufferBindings[i].Size = 0;
+ }
+ else {
+ ctx->AtomicBufferBindings[i].Offset = -1;
+ ctx->AtomicBufferBindings[i].Size = -1;
+ }
}
--
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] ARB_arrays_of_arrays GLSL ES
On Mon, 2015-06-22 at 19:04 +0300, Eero Tamminen wrote: > Hi, > > On 06/20/2015 03:32 PM, Timothy Arceri wrote: > > The restrictions in ES make the extension easier to implement so > > I thought I'd try get this stuff reviewed an committed before > > finishing > > up the full extension. > > The bits that I'm still working on for the desktop version are AoA > > inputs > > outputs, and interface blocks. > > > > The only thing I know is definatly missing in this series for ES is > > support for indirect indexing of samplers, but that didn't seem > > like > > something that should hold up the series. > > > > Once the SSBO series lands (with a patch that restricts unsized > > arrays) > > then all the AoA ES conformance tests will pass. > > > > There are already a bunch of piglit tests in git but I've just sent > > a > > series with all the patches still waiting review here: > > http://lists.freedesktop.org/archives/piglit/2015-June/016312.html > > > > I haven't made a patch marking this as done yet because currently > > the i965 backend takes a very long time trying to optimise some of > > the > > conformance tests. They still pass but they are taking 15-minutes+ > > just > > to compile so this really needs to be sorted out first. If someone > > with > > more knowledge in this area than me wants to take a look at this I > > would > > be greatful for being pointed in the right direction. > > Are there individual shaders which compilation take several minutes? Yes. The shaders are part of these tests: ES31-CTS.arrays_of_arrays.InteractionFunctionCalls1 ES31-CTS.arrays_of_arrays.InteractionFunctionCalls2 Callgrinds reporting 65% in vec4_live_variables::compute_live_variables() with InteractionFunctionCalls1 > > Do you have any perf [1] or valgrind [2] tool output for compiling > the > slowest one? > > > - Eero > > [1] # perf record -a > ^C > # perf report -n > -> text output > [2] $ valgrind --tool=callgrind > $ kcachegrind > -> callgraphs, callee maps etc > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 00/11] glapi fixes - build whole of mesa with
On 22/06/15 19:51, Emil Velikov wrote: On 22 June 2015 at 15:01, Jose Fonseca wrote: On 19/06/15 23:09, Emil Velikov wrote: On 19 June 2015 at 21:26, Jose Fonseca wrote: On 19/06/15 20:56, Emil Velikov wrote: Hi all, A lovely series inspired (more like 'was awaken to send these out') by Pal Rohár, who was having issues when building xlib-libgl (plus the now enabled gles*) So here, we teach the final two static glapi users about shared-glapi, plus some related fixes. After this is done we can finally start transitioning to shared-only glapi, with some more details as mentioned in one of the patches: XXX: With this one done, we can finally transition with enforcing shared-glapi, and - link the dri modules against libglapi.so, add --no-undefined to the LDFLAGS - drop the dlopen(libglapi.so/libGL.so, RTLD_GLOBAL) workarounds in the loaders - libGL, libEGL and libgbm. - start killing off/cleaning up the dispatch ? The caveats: 1) up to what stage do we care about static libraries - libgl (either dri or xlib based) - osmesa - libEGL 2) how about other platforms (scons) ? - currently the scons uses static glapi, - would we need the dlopen(...) on windows ? Hope everyone is excited about this one as I am :-) Maybe I missed the context of this changes, but why this matters or is an improvement? If one goes the extra mile (which this series doesn't) - one configure option less, substantial some code de-duplication and consistent use of the code amongst all components provided. This way any improvements/cleanups made to the shared glapi will be available to osmesa/xlib-libgl. I'm perfectly happy with removing the configure option. And I understand the benefits of unified code paths, but I believe that for this particular case, the difference in requirements really demands the separate code paths. In summary, having the ability of using a shared glapi sounds great, but forcing shared glapi everywhere, sounds a bad idea. I'm suspecting that people might be keen on the following idea - use static glapi for osmesa/xlib-libgl and shared one everywhere else? Yes, that sounds reasonable for me. (Needs libgl-gdi too.) Indeed. Everything gdi is build only via scons so we'll touch it only if needed. I fear that this will lead to further separation/bit-rot between the different implementations, but it seems like the bester compromise. I don't feel strongly between: a) using the same source code for both static/shared glapi (switched by a pre-processor define), or b) only share the interface but have shared/static glapi implementations. I'm actually not that familiar with that code. Either way, we can have two glapi build targets (a shared-glapi and a static-glapipe) side-by-side, so that there are no more source-wide configure flags. In theory it should be fine, in practise... I'm rather cautious as mapi is the most convoluted part in mesa, and with the "subdir-objects" option being toggled soon things may go (albeit unlikely) subtly haywire. I believe a lot of the complexity of that code comes from assembly. I wonder if it's really justified nowadays (and even if it is, whether it would be better served with GNU C assembly.) Futhermore, I believe on Windows we use any assembly, so if we split shared/static glapi source code, we could probably abandon assembly from the static-glapi. I'm not 100% sure but I'd suspect that Cygwin might use it when combined with swrast_dri. Don't know what others use - iirc some of the BSD folks are moving over to llvm. That I aside there is a massive amount of #ifdef spaghetti, apart from the assembly code. Can I have your ack/nack on the idea of having shared-glapi available for xlib-libgl (patches 2, 3 and 4), until we have both glapi's built in in parallel ? As mentioned originally, currently we fail to build if one enabled gles* and xlib-libgl and adding another hack in configure.ac is feel like flocking up a dead horse. I rarely use auto conf myself, but the mentioned 2-4 patches look OK to me. Acked-by: Jose Fonseca ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/6] mesa/es3.1: Do not allow zero size multisampled textures
On Tue, Jun 23, 2015 at 2:23 PM, Marta Lofstedt wrote: > From: Marta Lofstedt > > According to GLES 3.1 CTS test: > ES31-CTS.texture_storage_multisample. > APIGLTexStorage2DMultisample. > multisample_texture_tex_storage_2d_ > invalid_and_border_case_texture_sizes. > > Textures of size 0x0 are not allowed for > GL_TEXTURE_2D_MULTISAMPLE. > Please quote the spec rather than the CTS. The spec does define this, in section 8.8 "Multisample Textures": "An INVALID_VALUE error is generated if width or height is less than 1." > src/mesa/main/teximage.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c > index 3d85615..c76ad54 100644 > --- a/src/mesa/main/teximage.c > +++ b/src/mesa/main/teximage.c > @@ -1483,6 +1483,13 @@ _mesa_legal_texture_dimensions(struct gl_context *ctx, > GLenum target, > if (height > 0 && !_mesa_is_pow_two(height - 2 * border)) > return GL_FALSE; >} > + /* > + * according to GLES 3.1 CTS it is not OK with > + * zero size multisampled textures > + */ > + if (width == 0 && height == 0 && GL_TEXTURE_2D_MULTISAMPLE == target) > + return GL_FALSE; > + However, this is also the case for TexStorage2D, see section 8.17 "Immutable-Format Texture Images": "An INVALID_VALUE error is generated if width, height, depth or levels are less than 1, for commands with the corresponding parameters." So it seems this patch is somewhat incomplete, and only papers over a CTS failure. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/6] mesa/es3.1: Correct error code for illegal internal formats
On Tue, Jun 23, 2015 at 2:23 PM, Marta Lofstedt wrote: > From: Marta Lofstedt > > According to GLES 3.1 CTS test: > ES31-CTS.texture_storage_multisample. > APIGLTexStorage2DMultisample. > multisample_texture_tex_storage_2d_non_color_depth_or_stencil_ > internal_formats_receted. > > An illegal internal format should generate a > GL_INVALID_ENUM error. > OpenGL ES 3.1, section 8.8 defines this, not the CTS: "An INVALID_ENUM error is generated if sizedinternalformat is not colorrenderable, depth-renderable, or stencil-renderable (as defined in section 9.4)." ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC] gallium/hud: fix issue w/ tgsi_to_nir
From: Rob Clark
Ok, so actually there is a ttn issue here to fix as well.. but it
brought up a question in my mind. When ttn sees something like
DCL IN[0..1]
it will treat that as an array (which in the end will result in
constraints about where the registers get allocated. Which is not
really ideal.
With glsl we don't actually get input arrays (but instead a bunch
of MOV's to a TEMP array) currently. So I'm not quite sure how
an actual input array should look. (But my preference would be
IN[a..b] for arrays and IN[c] otherwise)
---
src/gallium/auxiliary/hud/hud_context.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/gallium/auxiliary/hud/hud_context.c
b/src/gallium/auxiliary/hud/hud_context.c
index 6a124f7..2b6d3a7 100644
--- a/src/gallium/auxiliary/hud/hud_context.c
+++ b/src/gallium/auxiliary/hud/hud_context.c
@@ -1163,7 +1163,8 @@ hud_create(struct pipe_context *pipe, struct cso_context
*cso)
{
static const char *vertex_shader_text = {
"VERT\n"
- "DCL IN[0..1]\n"
+ "DCL IN[0]\n"
+ "DCL IN[1]\n"
"DCL OUT[0], POSITION\n"
"DCL OUT[1], COLOR[0]\n" /* color */
"DCL OUT[2], GENERIC[0]\n" /* texcoord */
--
2.4.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/6] mesa/es3.1 : Correct error code for zero samples
On Tue, Jun 23, 2015 at 2:23 PM, Marta Lofstedt
wrote:
> From: Marta Lofstedt
>
> According to GLES 3.1 CTS test:
> ES31-CTS.texture_storage_multisample.
> APIGLTexStorage2DMultisample.
> multisample_texture_tex_storage_2d_zero_sample-
>
> A call to glTexStorageMultisample with target
> GL_TEXTURE_2D_MULTISAMPLE and zero samples,
> should return GL_INVALID_VALUE.
>
OpenGL ES 3.1, section 8.8:
"An INVALID_VALUE error is generated if samples is zero"
> However, with above the piglit test:
> gl-3.2-layered-rendering-framebuffertexture fails.
> Hence, only limit this for GLES 3.1 contexts.
>
OpenGL 4.5 say the following about TexStorage2DMultisample:
"Calling TexStorage2DMultisample is equivalent to calling
TexImage2DMultisample with the equivalently named parameters set to
the same values, except that the resulting texture has immutable
format."
and defines the following for TexImage*Multisample():
"An INVALID_VALUE error is generated if samples is zero"
This should cause the INVALID_VALUE error to happen for non-GLES. So
it seems either the Piglit test is wrong, or there's something more
going on. I'd expect the latter, due to some layered rendering
interaction. But I don't know for sure.
> Signed-off-by: Marta Lofstedt
> ---
> src/mesa/main/teximage.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> index 14af9cd..98f0223 100644
> --- a/src/mesa/main/teximage.c
> +++ b/src/mesa/main/teximage.c
> @@ -5588,6 +5588,16 @@ _mesa_texture_image_multisample(struct gl_context
> *ctx, GLuint dims,
> GLenum sample_count_error;
> bool dsa = strstr(func, "ture") ? true : false;
>
> + /*
> +* According to OpenGL ES 3.1 CTS zero samples
> +* should generate GL_INVALID_VALUE
> +*/
> + if(samples == 0 && _mesa_is_gles31(ctx))
> + {
> + _mesa_error(ctx, GL_INVALID_VALUE, "%s(target)", func);
> + return;
> + }
> +
> if (!(ctx->Extensions.ARB_texture_multisample
>&& _mesa_is_desktop_gl(ctx))) {
>_mesa_error(ctx, GL_INVALID_OPERATION, "%s(unsupported)", func);
> --
> 1.9.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/6] mesa/es3.1: Correct error code for illegal internal formats
On Tue, Jun 23, 2015 at 3:11 PM, Erik Faye-Lund wrote: > On Tue, Jun 23, 2015 at 2:23 PM, Marta Lofstedt > wrote: >> From: Marta Lofstedt >> >> According to GLES 3.1 CTS test: >> ES31-CTS.texture_storage_multisample. >> APIGLTexStorage2DMultisample. >> multisample_texture_tex_storage_2d_non_color_depth_or_stencil_ >> internal_formats_receted. >> >> An illegal internal format should generate a >> GL_INVALID_ENUM error. >> > > OpenGL ES 3.1, section 8.8 defines this, not the CTS: > > "An INVALID_ENUM error is generated if sizedinternalformat is not > colorrenderable, depth-renderable, or stencil-renderable (as defined > in section 9.4)." By the way, OpenGL 4.5 also defines this, so the patch should probably skip that condition. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/6] mesa/es3.1 : Correct error code for defect texture target
On Tue, Jun 23, 2015 at 2:23 PM, Marta Lofstedt
wrote:
> From: Marta Lofstedt
>
> According to GLES 3.1 CTS test:
> ES31-CTS.texture_storage_multisample.
> APIGLGetTexLevelParameterifv.
> invalid_texture_target_rejected:
>
> GL_INVALID_ENUM should be generated when
> glGetTexLevelParameteriv is called with a defect
> texture target.
>
Again, this is defined by the spec, not the CTS, section 8.10.3:
"An INVALID_ENUM error is generated if target is not one of the
texture targets described above"
But The OpenGL 4.5 spec defines the exact same error, so I don't think
we should check for gles3.1 here.
> Signed-off-by: Marta Lofstedt
> ---
> src/mesa/main/texobj.c | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/src/mesa/main/texobj.c b/src/mesa/main/texobj.c
> index c563f1e..c239deb 100644
> --- a/src/mesa/main/texobj.c
> +++ b/src/mesa/main/texobj.c
> @@ -222,6 +222,17 @@ _mesa_get_current_tex_object(struct gl_context *ctx,
> GLenum target)
> return ctx->Extensions.ARB_texture_multisample
> ? ctx->Texture.ProxyTex[TEXTURE_2D_MULTISAMPLE_ARRAY_INDEX] :
> NULL;
>default:
> + if(_mesa_is_gles31(ctx))
> + {
> +/*
> + * According to OpenGL ES 3.1 CTS:
> + *
> ES31-CTS.texture_storage_multisample.APIGLGetTexLevelParameterifv.
> + * invalid_value_argument_rejected
> + *
> es31cTextureStorageMultisampleGetTexLevelParameterifvTests.cpp:1277
> + * INVALID_ENUM should be reported for bad targets.
> + */
> +_mesa_error(ctx, GL_INVALID_ENUM, "%s(target)", __func__);
> + }
> _mesa_problem(NULL, "bad target in _mesa_get_current_tex_object()");
> return NULL;
> }
> --
> 1.9.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] tgsi: handle indirect sampler arrays. (v2)
Am 23.06.2015 um 07:53 schrieb Dave Airlie: > On 22 June 2015 at 21:20, Roland Scheidegger wrote: >> Should there be some clamping somewhere to prevent crashes due to >> out-of-bound unit index? > > The spec says its undefined, I'm never sure if that means explode in > any way whatsoever. > > Dave. > Yes, you are allowed to explode usually. But this is really crappy behavior which we try to avoid in general. And you're not allowed to explode with robust contexts. Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 13/14] st/mesa: set default tessellation levels
Another option would be to provide (no-op) implementations for all drivers. But I think generally we indeed check if the function is available in the state tracker before calling it for such things. Roland Am 23.06.2015 um 06:11 schrieb Ilia Mirkin: > This needs to be guarded on availability of tessellation. I'm guessing > that something ends up setting st.dirty to ~0, and this gets called > even when the driver doesn't support tess. Just hit this playing back > a trace with llvmpipe but with the tess patches: > > #1 0x737dcf49 in update_tess (st=0x7fffe8116530) > at state_tracker/st_atom_tess.c:46 > #2 0x737d7afb in st_validate_state (st=0x7fffe8116530) > at state_tracker/st_atom.c:223 > #3 0x737e4291 in st_Clear (ctx=0x7fffe80e1f10, mask=2) > at state_tracker/st_cb_clear.c:469 > #4 0x735f5c48 in _mesa_Clear (mask=16384) at main/clear.c:224 > #5 0x757ec452 in glClear (mask=16384) at glapi/glapi_mapi_tmp.h:3064 > #6 0x004d4c0f in retrace_glClear(trace::Call&) () > #7 0x0040f488 in retrace::Retracer::retrace(trace::Call&) () > > On Tue, Jun 16, 2015 at 7:04 PM, Marek Olšák wrote: >> From: Marek Olšák >> >> --- >> src/mesa/Makefile.sources | 1 + >> src/mesa/state_tracker/st_atom.c | 1 + >> src/mesa/state_tracker/st_atom.h | 1 + >> src/mesa/state_tracker/st_atom_tess.c | 59 >> +++ >> src/mesa/state_tracker/st_context.c | 1 + >> src/mesa/state_tracker/st_context.h | 2 +- >> 6 files changed, 64 insertions(+), 1 deletion(-) >> create mode 100644 src/mesa/state_tracker/st_atom_tess.c >> >> diff --git a/src/mesa/Makefile.sources b/src/mesa/Makefile.sources >> index 83f500f..ed9848c 100644 >> --- a/src/mesa/Makefile.sources >> +++ b/src/mesa/Makefile.sources >> @@ -407,6 +407,7 @@ STATETRACKER_FILES = \ >> state_tracker/st_atom_shader.c \ >> state_tracker/st_atom_shader.h \ >> state_tracker/st_atom_stipple.c \ >> + state_tracker/st_atom_tess.c \ >> state_tracker/st_atom_texture.c \ >> state_tracker/st_atom_viewport.c \ >> state_tracker/st_cache.h \ >> diff --git a/src/mesa/state_tracker/st_atom.c >> b/src/mesa/state_tracker/st_atom.c >> index c97cd84..5fc1a77 100644 >> --- a/src/mesa/state_tracker/st_atom.c >> +++ b/src/mesa/state_tracker/st_atom.c >> @@ -78,6 +78,7 @@ static const struct st_tracked_state *atoms[] = >> &st_bind_fs_ubos, >> &st_bind_gs_ubos, >> &st_update_pixel_transfer, >> + &st_update_tess, >> >> /* this must be done after the vertex program update */ >> &st_update_array >> diff --git a/src/mesa/state_tracker/st_atom.h >> b/src/mesa/state_tracker/st_atom.h >> index bbfbd2d..5735ca6 100644 >> --- a/src/mesa/state_tracker/st_atom.h >> +++ b/src/mesa/state_tracker/st_atom.h >> @@ -80,6 +80,7 @@ extern const struct st_tracked_state st_bind_gs_ubos; >> extern const struct st_tracked_state st_bind_tcs_ubos; >> extern const struct st_tracked_state st_bind_tes_ubos; >> extern const struct st_tracked_state st_update_pixel_transfer; >> +extern const struct st_tracked_state st_update_tess; >> >> >> GLuint st_compare_func_to_pipe(GLenum func); >> diff --git a/src/mesa/state_tracker/st_atom_tess.c >> b/src/mesa/state_tracker/st_atom_tess.c >> new file mode 100644 >> index 000..f3e >> --- /dev/null >> +++ b/src/mesa/state_tracker/st_atom_tess.c >> @@ -0,0 +1,59 @@ >> +/** >> + * >> + * Copyright 2015 Advanced Micro Devices, Inc. >> + * All Rights Reserved. >> + * >> + * Permission is hereby granted, free of charge, to any person obtaining a >> + * copy of this software and associated documentation files (the >> + * "Software"), to deal in the Software without restriction, including >> + * without limitation the rights to use, copy, modify, merge, publish, >> + * distribute, sub license, and/or sell copies of the Software, and to >> + * permit persons to whom the Software is furnished to do so, subject to >> + * the following conditions: >> + * >> + * The above copyright notice and this permission notice (including the >> + * next paragraph) shall be included in all copies or substantial portions >> + * of the Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS >> + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. >> + * IN NO EVENT SHALL THE AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR >> + * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, >> + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE >> + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. >> + * >> + **/ >> + >> +/* >> + * Authors: >> + * Marek Olšák >> + */ >> + >> + >> +#include "main/ma
Re: [Mesa-dev] [PATCH v3] glsl/es31:Allow GL_ARB_TEXTURE_MULTISAMPLE in GLSL ES 3.10
On Tue, Jun 23, 2015 at 3:39 AM, Marta Lofstedt
wrote:
> From: Marta Lofstedt
>
> Signed-off-by: Marta Lofstedt
> ---
> src/glsl/builtin_functions.cpp | 3 +--
> src/glsl/builtin_types.cpp | 2 +-
> src/glsl/glsl_lexer.ll | 13 +++--
> src/glsl/glsl_parser_extras.h | 5 +
> 4 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp
> index efab299..593a575 100644
> --- a/src/glsl/builtin_functions.cpp
> +++ b/src/glsl/builtin_functions.cpp
> @@ -270,8 +270,7 @@ texture_array(const _mesa_glsl_parse_state *state)
> static bool
> texture_multisample(const _mesa_glsl_parse_state *state)
> {
> - return state->is_version(150, 0) ||
> - state->ARB_texture_multisample_enable;
> + return state->has_texture_multisample();
> }
>
> static bool
> diff --git a/src/glsl/builtin_types.cpp b/src/glsl/builtin_types.cpp
> index d92e2eb..9968f7c 100644
> --- a/src/glsl/builtin_types.cpp
> +++ b/src/glsl/builtin_types.cpp
> @@ -307,7 +307,7 @@ _mesa_glsl_initialize_types(struct _mesa_glsl_parse_state
> *state)
>add_type(symbols, glsl_type::usamplerCubeArray_type);
> }
>
> - if (state->ARB_texture_multisample_enable) {
> + if (state->has_texture_multisample()) {
Originally I thought this was a bugfix, but on closer examination, you
should just update the table above, builtin_type_versions. Currently
it has:
T(sampler2DMS, 150, 999)
T(sampler2DMSArray,150, 999)
and so on for the int/uint variants. Change the 999 to 310 and you
should be good to go.
>add_type(symbols, glsl_type::sampler2DMS_type);
>add_type(symbols, glsl_type::isampler2DMS_type);
>add_type(symbols, glsl_type::usampler2DMS_type);
> diff --git a/src/glsl/glsl_lexer.ll b/src/glsl/glsl_lexer.ll
> index 10db5b8..3597435 100644
> --- a/src/glsl/glsl_lexer.ll
> +++ b/src/glsl/glsl_lexer.ll
> @@ -341,12 +341,13 @@ usampler2DArray KEYWORD(130, 300, 130, 300,
> USAMPLER2DARRAY);
>
> /* additional keywords in ARB_texture_multisample, included in GLSL 1.50
> */
> /* these are reserved but not defined in GLSL 3.00 */
> -sampler2DMSKEYWORD_WITH_ALT(150, 300, 150, 0,
> yyextra->ARB_texture_multisample_enable, SAMPLER2DMS);
> -isampler2DMS KEYWORD_WITH_ALT(150, 300, 150, 0,
> yyextra->ARB_texture_multisample_enable, ISAMPLER2DMS);
> -usampler2DMS KEYWORD_WITH_ALT(150, 300, 150, 0,
> yyextra->ARB_texture_multisample_enable, USAMPLER2DMS);
> -sampler2DMSArray KEYWORD_WITH_ALT(150, 300, 150, 0,
> yyextra->ARB_texture_multisample_enable, SAMPLER2DMSARRAY);
> -isampler2DMSArray KEYWORD_WITH_ALT(150, 300, 150, 0,
> yyextra->ARB_texture_multisample_enable, ISAMPLER2DMSARRAY);
> -usampler2DMSArray KEYWORD_WITH_ALT(150, 300, 150, 0,
> yyextra->ARB_texture_multisample_enable, USAMPLER2DMSARRAY);
> + /* these are needed for GLES 3.1 */
> +sampler2DMSKEYWORD_WITH_ALT(150, 300, 150, 310,
> yyextra->ARB_texture_multisample_enable, SAMPLER2DMS);
> +isampler2DMS KEYWORD_WITH_ALT(150, 300, 150, 310,
> yyextra->ARB_texture_multisample_enable, ISAMPLER2DMS);
> +usampler2DMS KEYWORD_WITH_ALT(150, 300, 150, 310,
> yyextra->ARB_texture_multisample_enable, USAMPLER2DMS);
> +sampler2DMSArray KEYWORD_WITH_ALT(150, 300, 150, 310,
> yyextra->ARB_texture_multisample_enable, SAMPLER2DMSARRAY);
> +isampler2DMSArray KEYWORD_WITH_ALT(150, 300, 150, 310,
> yyextra->ARB_texture_multisample_enable, ISAMPLER2DMSARRAY);
> +usampler2DMSArray KEYWORD_WITH_ALT(150, 300, 150, 310,
> yyextra->ARB_texture_multisample_enable, USAMPLER2DMSARRAY);
>
> /* keywords available with ARB_texture_cube_map_array_enable extension on
> desktop GLSL */
> samplerCubeArray KEYWORD_WITH_ALT(400, 0, 400, 0,
> yyextra->ARB_texture_cube_map_array_enable, SAMPLERCUBEARRAY);
> diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
> index 9a0c24e..a231d96 100644
> --- a/src/glsl/glsl_parser_extras.h
> +++ b/src/glsl/glsl_parser_extras.h
> @@ -221,6 +221,11 @@ struct _mesa_glsl_parse_state {
> || EXT_separate_shader_objects_enable;
> }
>
> + bool has_texture_multisample() const
> + {
> + return ARB_texture_multisample_enable || is_version(150, 310);
> + }
Since the other use isn't necessary, I'd also remove this and just fix
up the original texture_multisample() function to know about ES310
> +
> bool has_double() const
> {
>return ARB_gpu_shader_fp64_enable || is_version(400, 0);
> --
> 1.9.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 5/6] mesa/es31: AtomicBufferBindings should be initialized to zero.
On Tue, Jun 23, 2015 at 2:23 PM, Marta Lofstedt
wrote:
> From: Marta Lofstedt
>
> Accoring to GLES 3.1 CTS:
> GLES 3.1 CTS: ES31-CTS.shader_atomic_counters.
> basic-buffer-bind.
>
> AtomicBufferBindings size and start should be initialized
> to zero.
>
OpenGL 3.1 says:
"Buffer variables in shader storage blocks are represented in memory
in the same way as uniforms stored in uniform blocks, as described in
section 7.6.2.1."
Table 6.2, "Buffer object parameters and their values" defines what
seems like the initial values for this. And OpenGL 4.5 defines the
same.
But I guess someone who knows atomic buffers better than me should clarify.
> Signed-off-by: Marta Lofstedt
> ---
> src/mesa/main/bufferobj.c | 11 ---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
> index 66dee68..94629b3 100644
> --- a/src/mesa/main/bufferobj.c
> +++ b/src/mesa/main/bufferobj.c
> @@ -849,9 +849,14 @@ _mesa_init_buffer_objects( struct gl_context *ctx )
>_mesa_reference_buffer_object(ctx,
>
> &ctx->AtomicBufferBindings[i].BufferObject,
> ctx->Shared->NullBufferObj);
> - ctx->AtomicBufferBindings[i].Offset = -1;
> - ctx->AtomicBufferBindings[i].Size = -1;
> - }
> + if (_mesa_is_gles31(ctx)) {
> + ctx->AtomicBufferBindings[i].Offset = 0;
> + ctx->AtomicBufferBindings[i].Size = 0;
> + }
> + else {
> + ctx->AtomicBufferBindings[i].Offset = -1;
> + ctx->AtomicBufferBindings[i].Size = -1;
> + }
> }
>
>
> --
> 1.9.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 6/6] mesa/es3.1: Fix error code for glCreateShaderProgram
On Tue, Jun 23, 2015 at 2:23 PM, Marta Lofstedt wrote: > From: Marta Lofstedt > > According to the OpenGL ES standard, 7.3. > For a call to glCreateShaderProgram with count < 0, > a GL_INVALID_VALUE error should be generated. > OpenGL 4.5 defines it the same way. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2] mesa/es3.1: Fix error code for glCreateShaderProgram
From: Marta Lofstedt
According to the OpenGL ES standard, 7.3.
For a call to glCreateShaderProgram with count < 0,
a GL_INVALID_VALUE error should be generated.
Signed-off-by: Marta Lofstedt
---
src/mesa/main/shaderapi.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index c783c69..266064d 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -1890,6 +1890,15 @@ _mesa_create_shader_program(struct gl_context* ctx,
GLboolean separate,
const GLuint shader = create_shader(ctx, type);
GLuint program = 0;
+ /*
+* According to OpenGL ES 3.1 standard 7.3: GL_INVALID_VALUE
+* should be generated, if count < 0.
+*/
+ if (_mesa_is_gles31(ctx) && count < 0) {
+ _mesa_error(ctx, GL_INVALID_VALUE, "glCreateShaderProgram (count < 0)");
+ return program;
+ }
+
if (shader) {
_mesa_ShaderSource(shader, count, strings, NULL);
--
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] glsl: Fix counting of varyings.
On 22/06/15 17:14, Ian Romanick wrote:
On 06/19/2015 06:08 AM, Jose Fonseca wrote:
When input and output varyings started to be counted separately (commit
42305fb5) the is_varying_var function wasn't updated to return true for
output varyings or input varyings for stages other than the fragment
shader), effectively making the varying limit to never be checked.
Without SSO, counting the varying inputs used by, say, the fragment
shader, should be sufficient. With SSO, it's more difficult.
With this change, color, texture coord, and generic varyings are not
counted, but others are ignored. It is assumed the hardware will handle
special varyings internally (ie, optimistic rather than pessimistic), to
avoid causing regressions where things were working somehow.
This fixes `glsl-max-varyings --exceed-limits` with softpipe/llvmpipe,
which was asserting because we were getting varyings beyond
VARYING_SLOT_MAX in st_glsl_to_tgsi.cpp.
It also prevents the assertion failure with
https://bugs.freedesktop.org/show_bug.cgi?id=90539 but the tests still
fails due to the link error.
This change also adds a few assertions to catch this sort of errors
earlier, and potentially prevent buffer overflows in the future (no
buffer overflow was detected here though).
However, this change causes several tests to regress:
spec/glsl-1.10/execution/varying-packing/simple ivec3 array
spec/glsl-1.10/execution/varying-packing/simple ivec3 separate
spec/glsl-1.10/execution/varying-packing/simple uvec3 array
spec/glsl-1.10/execution/varying-packing/simple uvec3 separate
Wait... so the ivec3 and uvec3 tests fail, but the vec3 test passes?
Correct. This is partial diff from vec3 vs ivec3's GLSL:
: GLSL source for vertex shader 1:
-: #version 110
-varying vec3 var000[42];
-varying float var001;
-varying float var002;
+: #version 130
+flat out ivec3 var000[42];
+out float var001;
+out float var002;
uniform int i;
And it looks like the varying packer refuses the pack together variables
of different types.
Not sure this is a bug in the test, or a limitation in the varying
packing pass. Either way, it's a bug that was being hidden and needs to
be addressed.
spec/arb_gpu_shader_fp64/varying-packing/simple dmat3 array
spec/glsl-1.50/execution/geometry/max-input-components
spec/glsl-1.50/execution/variable-indexing/gs-input-array-vec4-index-rd
spec/glsl-1.50/execution/variable-indexing/vs-output-array-vec4-index-wr-before-gs
But this all seem to be issues either in the way we count varyings
(e.g., geometry inputs get counted multiple times) or in the tests
themselves, or limitations in the varying packer, and deserve attention
on their own right.
Do you have a feeling for which tests are which sorts of problems?
Only a rough idea:
- The "varying-packing/simple" failures look all similar in nature to
what I described above, ie., int, uint, or doubles not being packed with
floats.
- the geometry related ones are because the code to count GS varyings
over-estimates the varyings (it counts the varyings for the whole
primitive, not just a single vertex)
but I workaround this for now in my change, by returning 0 for GS
(ie, no change for GS).
I'd like to run this through GLES3 conformance before it gets pushed.
I'm not too worried about the geometry shader issues, but the ivec /
uvec tests seem more problematic.
Sure.
---
src/glsl/link_varyings.cpp | 70 --
src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 2 +
2 files changed, 58 insertions(+), 14 deletions(-)
diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
index 278a778..7649720 100644
--- a/src/glsl/link_varyings.cpp
+++ b/src/glsl/link_varyings.cpp
@@ -190,6 +190,8 @@ cross_validate_outputs_to_inputs(struct gl_shader_program
*prog,
*/
const unsigned idx = var->data.location - VARYING_SLOT_VAR0;
+ assert(idx < MAX_VARYING);
+
if (explicit_locations[idx] != NULL) {
linker_error(prog,
"%s shader has multiple outputs explicitly "
@@ -1031,25 +1033,63 @@ varying_matches::match_comparator(const void
*x_generic, const void *y_generic)
/**
* Is the given variable a varying variable to be counted against the
* limit in ctx->Const.MaxVarying?
- * This includes variables such as texcoords, colors and generic
- * varyings, but excludes variables such as gl_FrontFacing and gl_FragCoord.
+ *
+ * OpenGL specification states:
Please use the canonical format.
* Section A.B (Foo Bar) of the OpenGL X.Y Whichever Profile spec
* says:
That enables later readers to more easily find the text in the spec.
Also, the language changes from time to time.
+ *
+ * Each output variable component used as either a vertex shader output or
+ * fragment shader input counts against this limit, except for the components
+ * of gl_Position. A program containing only a vertex an
Re: [Mesa-dev] [PATCH 07/17] i965/fs: Move offset() and half() to the fs_builder
On Thu, 2015-06-18 at 17:51 -0700, Jason Ekstrand wrote:
> We want to move these into the builder so that they know the current
> builder's dispatch width. This will be needed by a later commit.
> ---
> src/mesa/drivers/dri/i965/brw_fs.cpp | 52 ++
> src/mesa/drivers/dri/i965/brw_fs_builder.h | 46 +
> src/mesa/drivers/dri/i965/brw_fs_cse.cpp | 2 +-
> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 60 +--
> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 149
> ++-
> src/mesa/drivers/dri/i965/brw_ir_fs.h| 51 -
> 6 files changed, 182 insertions(+), 178 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 4f98d63..c13ac7d 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -267,7 +267,7 @@ fs_visitor::VARYING_PULL_CONSTANT_LOAD(const fs_builder
> &bld,
> inst->mlen = 1 + dispatch_width / 8;
> }
>
> - bld.MOV(dst, offset(vec4_result, (const_offset & 3) * scale));
> + bld.MOV(dst, bld.offset(vec4_result, (const_offset & 3) * scale));
> }
>
> /**
> @@ -361,7 +361,12 @@ fs_inst::is_copy_payload(const brw::simple_allocator
> &grf_alloc) const
>reg.width = this->src[i].width;
>if (!this->src[i].equals(reg))
> return false;
> - reg = ::offset(reg, 1);
> +
> + if (i < this->header_size) {
> + reg.reg_offset += 1;
> + } else {
> + reg.reg_offset += this->exec_size / 8;
> + }
This here looks like you are squashing a fix. Should it go in a separate
patch?
> }
>
> return true;
> @@ -963,7 +968,7 @@ fs_visitor::emit_fragcoord_interpolation(bool
> pixel_center_integer,
> } else {
>bld.ADD(wpos, this->pixel_x, fs_reg(0.5f));
> }
> - wpos = offset(wpos, 1);
> + wpos = bld.offset(wpos, 1);
>
> /* gl_FragCoord.y */
> if (!flip && pixel_center_integer) {
> @@ -979,7 +984,7 @@ fs_visitor::emit_fragcoord_interpolation(bool
> pixel_center_integer,
>
>bld.ADD(wpos, pixel_y, fs_reg(offset));
> }
> - wpos = offset(wpos, 1);
> + wpos = bld.offset(wpos, 1);
>
> /* gl_FragCoord.z */
> if (devinfo->gen >= 6) {
> @@ -989,7 +994,7 @@ fs_visitor::emit_fragcoord_interpolation(bool
> pixel_center_integer,
> this->delta_xy[BRW_WM_PERSPECTIVE_PIXEL_BARYCENTRIC],
> interp_reg(VARYING_SLOT_POS, 2));
> }
> - wpos = offset(wpos, 1);
> + wpos = bld.offset(wpos, 1);
>
> /* gl_FragCoord.w: Already set up in emit_interpolation */
> bld.MOV(wpos, this->wpos_w);
> @@ -1072,7 +1077,7 @@ fs_visitor::emit_general_interpolation(fs_reg attr,
> const char *name,
> /* If there's no incoming setup data for this slot, don't
>* emit interpolation for it.
>*/
> - attr = offset(attr, type->vector_elements);
> + attr = bld.offset(attr, type->vector_elements);
> location++;
> continue;
>}
> @@ -1087,7 +1092,7 @@ fs_visitor::emit_general_interpolation(fs_reg attr,
> const char *name,
> interp = suboffset(interp, 3);
> interp.type = attr.type;
> bld.emit(FS_OPCODE_CINTERP, attr, fs_reg(interp));
> -attr = offset(attr, 1);
> +attr = bld.offset(attr, 1);
> }
>} else {
> /* Smooth/noperspective interpolation case. */
> @@ -1125,7 +1130,7 @@ fs_visitor::emit_general_interpolation(fs_reg attr,
> const char *name,
> if (devinfo->gen < 6 && interpolation_mode ==
> INTERP_QUALIFIER_SMOOTH) {
>bld.MUL(attr, attr, this->pixel_w);
> }
> -attr = offset(attr, 1);
> +attr = bld.offset(attr, 1);
> }
>
>}
> @@ -1227,19 +1232,19 @@ fs_visitor::emit_samplepos_setup()
> if (dispatch_width == 8) {
>abld.MOV(int_sample_x, fs_reg(sample_pos_reg));
> } else {
> - abld.half(0).MOV(half(int_sample_x, 0), fs_reg(sample_pos_reg));
> - abld.half(1).MOV(half(int_sample_x, 1),
> + abld.half(0).MOV(abld.half(int_sample_x, 0), fs_reg(sample_pos_reg));
> + abld.half(1).MOV(abld.half(int_sample_x, 1),
> fs_reg(suboffset(sample_pos_reg, 16)));
> }
> /* Compute gl_SamplePosition.x */
> compute_sample_position(pos, int_sample_x);
> - pos = offset(pos, 1);
> + pos = abld.offset(pos, 1);
> if (dispatch_width == 8) {
>abld.MOV(int_sample_y, fs_reg(suboffset(sample_pos_reg, 1)));
> } else {
> - abld.half(0).MOV(half(int_sample_y, 0),
> + abld.half(0).MOV(abld.half(int_sample_y, 0),
> fs_reg(suboffset(sample_pos_reg, 1)));
> - abld.half(1).MOV(half(int_sample_y, 1),
> + abld.half(1).MOV(abld.half(int_sample_y, 1),
> fs_reg(suboffset(sample_pos_reg, 17)));
> }
> /* Compute gl_Sa
Re: [Mesa-dev] [PATCH 2/2] glsl: Fix counting of varyings.
On 23/06/15 15:36, Jose Fonseca wrote:
On 22/06/15 17:14, Ian Romanick wrote:
On 06/19/2015 06:08 AM, Jose Fonseca wrote:
When input and output varyings started to be counted separately (commit
42305fb5) the is_varying_var function wasn't updated to return true for
output varyings or input varyings for stages other than the fragment
shader), effectively making the varying limit to never be checked.
Without SSO, counting the varying inputs used by, say, the fragment
shader, should be sufficient. With SSO, it's more difficult.
With this change, color, texture coord, and generic varyings are not
counted, but others are ignored. It is assumed the hardware will handle
special varyings internally (ie, optimistic rather than pessimistic), to
avoid causing regressions where things were working somehow.
This fixes `glsl-max-varyings --exceed-limits` with softpipe/llvmpipe,
which was asserting because we were getting varyings beyond
VARYING_SLOT_MAX in st_glsl_to_tgsi.cpp.
It also prevents the assertion failure with
https://bugs.freedesktop.org/show_bug.cgi?id=90539 but the tests still
fails due to the link error.
This change also adds a few assertions to catch this sort of errors
earlier, and potentially prevent buffer overflows in the future (no
buffer overflow was detected here though).
However, this change causes several tests to regress:
spec/glsl-1.10/execution/varying-packing/simple ivec3 array
spec/glsl-1.10/execution/varying-packing/simple ivec3 separate
spec/glsl-1.10/execution/varying-packing/simple uvec3 array
spec/glsl-1.10/execution/varying-packing/simple uvec3 separate
Wait... so the ivec3 and uvec3 tests fail, but the vec3 test passes?
Correct. This is partial diff from vec3 vs ivec3's GLSL:
: GLSL source for vertex shader 1:
-: #version 110
-varying vec3 var000[42];
-varying float var001;
-varying float var002;
+: #version 130
+flat out ivec3 var000[42];
+out float var001;
+out float var002;
uniform int i;
And it looks like the varying packer refuses the pack together variables
of different types.
Not sure this is a bug in the test, or a limitation in the varying
packing pass. Either way, it's a bug that was being hidden and needs to
be addressed.
spec/arb_gpu_shader_fp64/varying-packing/simple dmat3 array
spec/glsl-1.50/execution/geometry/max-input-components
spec/glsl-1.50/execution/variable-indexing/gs-input-array-vec4-index-rd
spec/glsl-1.50/execution/variable-indexing/vs-output-array-vec4-index-wr-before-gs
But this all seem to be issues either in the way we count varyings
(e.g., geometry inputs get counted multiple times) or in the tests
themselves, or limitations in the varying packer, and deserve attention
on their own right.
Do you have a feeling for which tests are which sorts of problems?
Only a rough idea:
- The "varying-packing/simple" failures look all similar in nature to
what I described above, ie., int, uint, or doubles not being packed with
floats.
- the geometry related ones are because the code to count GS varyings
over-estimates the varyings (it counts the varyings for the whole
primitive, not just a single vertex)
but I workaround this for now in my change, by returning 0 for GS
(ie, no change for GS).
I'd like to run this through GLES3 conformance before it gets pushed.
I'm not too worried about the geometry shader issues, but the ivec /
uvec tests seem more problematic.
Sure.
---
src/glsl/link_varyings.cpp | 70
--
src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 2 +
2 files changed, 58 insertions(+), 14 deletions(-)
diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
index 278a778..7649720 100644
--- a/src/glsl/link_varyings.cpp
+++ b/src/glsl/link_varyings.cpp
@@ -190,6 +190,8 @@ cross_validate_outputs_to_inputs(struct
gl_shader_program *prog,
*/
const unsigned idx = var->data.location - VARYING_SLOT_VAR0;
+ assert(idx < MAX_VARYING);
+
if (explicit_locations[idx] != NULL) {
linker_error(prog,
"%s shader has multiple outputs explicitly "
@@ -1031,25 +1033,63 @@ varying_matches::match_comparator(const void
*x_generic, const void *y_generic)
/**
* Is the given variable a varying variable to be counted against the
* limit in ctx->Const.MaxVarying?
- * This includes variables such as texcoords, colors and generic
- * varyings, but excludes variables such as gl_FrontFacing and
gl_FragCoord.
+ *
+ * OpenGL specification states:
Please use the canonical format.
* Section A.B (Foo Bar) of the OpenGL X.Y Whichever Profile spec
* says:
That enables later readers to more easily find the text in the spec.
Also, the language changes from time to time.
+ *
+ * Each output variable component used as either a vertex shader
output or
+ * fragment shader input counts against this limit, except for the
components
+ * of gl_Position. A program con
Re: [Mesa-dev] [Mesa-stable] [PATCH] bugzilla_mesa.sh: sort the bugs list by number
On Fri 19 Jun 2015, Emil Velikov wrote: > Cc: "10.5 10.6" > Suggested-by: Ilia Mirkin > Signed-off-by: Emil Velikov > -urls=$(git log $* | grep 'bugs.freedesktop.org/show_bug' | sed -e > $trim_before -e $trim_after -e $use_https | sort | uniq) > +urls=$(git log $* | grep 'bugs.freedesktop.org/show_bug' | sed -e > $trim_before -e $trim_after -e $use_https | sort-n | uniq) That can't be right! A space is needed between 'sort' and '-n'. Add the space, and this is Reviewed-by: Chad Versace ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] bugzilla_mesa.sh: sort the bugs list by number
On Tue, Jun 23, 2015 at 12:05 PM, Chad Versace wrote: > On Fri 19 Jun 2015, Emil Velikov wrote: >> Cc: "10.5 10.6" >> Suggested-by: Ilia Mirkin >> Signed-off-by: Emil Velikov > > >> -urls=$(git log $* | grep 'bugs.freedesktop.org/show_bug' | sed -e >> $trim_before -e $trim_after -e $use_https | sort | uniq) >> +urls=$(git log $* | grep 'bugs.freedesktop.org/show_bug' | sed -e >> $trim_before -e $trim_after -e $use_https | sort-n | uniq) > > That can't be right! A space is needed between 'sort' and '-n'. The approach is flawed... sort -n expects the number first, not in the middle/last. This was my suggestion to Emil: git log $* | grep 'bugs.freedesktop.org/show_bug' | sed -e 's/.*show_bug.cgi?id=\([0-9]*\).*/\1/' | sort -n -u | sed 's,^,https://bugs.freedesktop.org/show_bug.cgi?id=,' > > Add the space, and this is > Reviewed-by: Chad Versace > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 07/17] i965/fs: Move offset() and half() to the fs_builder
Jason Ekstrand writes:
> We want to move these into the builder so that they know the current
> builder's dispatch width. This will be needed by a later commit.
I very much like the idea of this series, but, why do you need to move
these register manipulators into the builder? The builder is an object
you can use to:
- Manipulate and query parameters affecting code generation.
- Create instructions into the program (::emit and friends).
- Allocate virtual registers from the program (::vgrf and friends).
offset() and half() logically perform an action on a given register
object (or rather, compute a function of a given register object), not
on a builder object, the builder is only required as an auxiliary
parameter -- Any reason you didn't just pass it as a third parameter?
As offset() and half() don't require access to any private details of
the builder, that would actually improve encapsulation, and would avoid
the dubious overloading of fs_builder::half() with two methods with
completely different semantics.
Thanks.
> ---
> src/mesa/drivers/dri/i965/brw_fs.cpp | 52 ++
> src/mesa/drivers/dri/i965/brw_fs_builder.h | 46 +
> src/mesa/drivers/dri/i965/brw_fs_cse.cpp | 2 +-
> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 60 +--
> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 149
> ++-
> src/mesa/drivers/dri/i965/brw_ir_fs.h| 51 -
> 6 files changed, 182 insertions(+), 178 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 4f98d63..c13ac7d 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -267,7 +267,7 @@ fs_visitor::VARYING_PULL_CONSTANT_LOAD(const fs_builder
> &bld,
> inst->mlen = 1 + dispatch_width / 8;
> }
>
> - bld.MOV(dst, offset(vec4_result, (const_offset & 3) * scale));
> + bld.MOV(dst, bld.offset(vec4_result, (const_offset & 3) * scale));
> }
>
> /**
> @@ -361,7 +361,12 @@ fs_inst::is_copy_payload(const brw::simple_allocator
> &grf_alloc) const
>reg.width = this->src[i].width;
>if (!this->src[i].equals(reg))
> return false;
> - reg = ::offset(reg, 1);
> +
> + if (i < this->header_size) {
> + reg.reg_offset += 1;
> + } else {
> + reg.reg_offset += this->exec_size / 8;
> + }
> }
>
> return true;
> @@ -963,7 +968,7 @@ fs_visitor::emit_fragcoord_interpolation(bool
> pixel_center_integer,
> } else {
>bld.ADD(wpos, this->pixel_x, fs_reg(0.5f));
> }
> - wpos = offset(wpos, 1);
> + wpos = bld.offset(wpos, 1);
>
> /* gl_FragCoord.y */
> if (!flip && pixel_center_integer) {
> @@ -979,7 +984,7 @@ fs_visitor::emit_fragcoord_interpolation(bool
> pixel_center_integer,
>
>bld.ADD(wpos, pixel_y, fs_reg(offset));
> }
> - wpos = offset(wpos, 1);
> + wpos = bld.offset(wpos, 1);
>
> /* gl_FragCoord.z */
> if (devinfo->gen >= 6) {
> @@ -989,7 +994,7 @@ fs_visitor::emit_fragcoord_interpolation(bool
> pixel_center_integer,
> this->delta_xy[BRW_WM_PERSPECTIVE_PIXEL_BARYCENTRIC],
> interp_reg(VARYING_SLOT_POS, 2));
> }
> - wpos = offset(wpos, 1);
> + wpos = bld.offset(wpos, 1);
>
> /* gl_FragCoord.w: Already set up in emit_interpolation */
> bld.MOV(wpos, this->wpos_w);
> @@ -1072,7 +1077,7 @@ fs_visitor::emit_general_interpolation(fs_reg attr,
> const char *name,
> /* If there's no incoming setup data for this slot, don't
>* emit interpolation for it.
>*/
> - attr = offset(attr, type->vector_elements);
> + attr = bld.offset(attr, type->vector_elements);
> location++;
> continue;
>}
> @@ -1087,7 +1092,7 @@ fs_visitor::emit_general_interpolation(fs_reg attr,
> const char *name,
> interp = suboffset(interp, 3);
> interp.type = attr.type;
> bld.emit(FS_OPCODE_CINTERP, attr, fs_reg(interp));
> -attr = offset(attr, 1);
> +attr = bld.offset(attr, 1);
> }
>} else {
> /* Smooth/noperspective interpolation case. */
> @@ -1125,7 +1130,7 @@ fs_visitor::emit_general_interpolation(fs_reg attr,
> const char *name,
> if (devinfo->gen < 6 && interpolation_mode ==
> INTERP_QUALIFIER_SMOOTH) {
>bld.MUL(attr, attr, this->pixel_w);
> }
> -attr = offset(attr, 1);
> +attr = bld.offset(attr, 1);
> }
>
>}
> @@ -1227,19 +1232,19 @@ fs_visitor::emit_samplepos_setup()
> if (dispatch_width == 8) {
>abld.MOV(int_sample_x, fs_reg(sample_pos_reg));
> } else {
> - abld.half(0).MOV(half(int_sample_x, 0), fs_reg(sample_pos_reg));
> - abld.half(1).MOV(half(int_sample_x, 1),
> + abld.half(0).MOV(abld.half(int_sample_x, 0), fs_reg(sample_pos_reg))
[Mesa-dev] [PATCH] st/mesa: remove unneeded pipe_surface_release() in st_render_texture()
This caused us to always free the pipe_surface for the renderbuffer. The subsequent call to st_update_renderbuffer_surface() would typically just recreate it. Remove the call to pipe_surface_release() and let st_update_renderbuffer_surface() take care of freeing the old surface if it needs to be replaced (because of change to mipmap level, etc). This can save quite a few calls to pipe_context::create_surface() and surface_destroy(). --- src/mesa/state_tracker/st_cb_fbo.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/mesa/state_tracker/st_cb_fbo.c b/src/mesa/state_tracker/st_cb_fbo.c index 0399eef..5707590 100644 --- a/src/mesa/state_tracker/st_cb_fbo.c +++ b/src/mesa/state_tracker/st_cb_fbo.c @@ -511,8 +511,6 @@ st_render_texture(struct gl_context *ctx, strb->rtt_layered = att->Layered; pipe_resource_reference(&strb->texture, pt); - pipe_surface_release(pipe, &strb->surface); - st_update_renderbuffer_surface(st, strb); strb->Base.Format = st_pipe_format_to_mesa_format(pt->format); -- 1.9.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] bugzilla_mesa.sh: sort the bugs list by number
On Tue 23 Jun 2015, Ilia Mirkin wrote: > On Tue, Jun 23, 2015 at 12:05 PM, Chad Versace wrote: > > On Fri 19 Jun 2015, Emil Velikov wrote: > >> Cc: "10.5 10.6" > >> Suggested-by: Ilia Mirkin > >> Signed-off-by: Emil Velikov > > > > > >> -urls=$(git log $* | grep 'bugs.freedesktop.org/show_bug' | sed -e > >> $trim_before -e $trim_after -e $use_https | sort | uniq) > >> +urls=$(git log $* | grep 'bugs.freedesktop.org/show_bug' | sed -e > >> $trim_before -e $trim_after -e $use_https | sort-n | uniq) > > > > That can't be right! A space is needed between 'sort' and '-n'. > > The approach is flawed... sort -n expects the number first, not in the > middle/last. This was my suggestion to Emil: > > git log $* | grep 'bugs.freedesktop.org/show_bug' | sed -e > 's/.*show_bug.cgi?id=\([0-9]*\).*/\1/' | sort -n -u | sed > 's,^,https://bugs.freedesktop.org/show_bug.cgi?id=,' Right. I withdraw my rb. FYI, sed isn't needed, at least on Linux. Linux's 'sort' knows how to sort on arbitrary fields and separators. This worked for me: -urls=$(git log $* | grep 'bugs.freedesktop.org/show_bug' | sed -e $trim_before -e $trim_after -e $use_https | sort | uniq) +urls=$(git log $* | grep 'bugs.freedesktop.org/show_bug' | sed -e $trim_before -e $trim_after -e $use_https | sort -n -u -k2 -t=) Or, with long options: -urls=$(git log $* | grep 'bugs.freedesktop.org/show_bug' | sed -e $trim_before -e $trim_after -e $use_https | sort | uniq) +urls=$(git log $* | grep 'bugs.freedesktop.org/show_bug' | sed -e $trim_before -e $trim_after -e $use_https | sort -n --unique --key=2 --field-separator==) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 91044] piglit spec/egl_khr_create_context/valid debug flag gles* fail
https://bugs.freedesktop.org/show_bug.cgi?id=91044 --- Comment #4 from Emil Velikov --- Got confused there thinking that you're with Intel, sorry about that Boyan. As expected we're missing the v15 update of the spec in our libEGL. I will try to find some time and dig through the spec history late this week. This way we'll clearly see if there are other missing pieces ;-) And yes, allowing EGL_CONTEXT_OPENGL_DEBUG_BIT_KHR for GLES seems like the correct thing to do here (as per the spec quote below) If the EGL_CONTEXT_OPENGL_DEBUG_BIT_KHR flag bit is set in EGL_CONTEXT_FLAGS_KHR, then a will be created... ... This bit is supported for OpenGL and OpenGL ES contexts. -- 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 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 05/16] i965: Move INTEL_DEBUG variable parsing to screen creation time
On Monday, June 22, 2015 06:07:25 PM Jason Ekstrand wrote:
> ---
> src/mesa/drivers/dri/i965/brw_context.c | 10 +-
> src/mesa/drivers/dri/i965/intel_debug.c | 13 ++---
> src/mesa/drivers/dri/i965/intel_debug.h | 4 ++--
> src/mesa/drivers/dri/i965/intel_screen.c | 2 ++
> 4 files changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c
> b/src/mesa/drivers/dri/i965/brw_context.c
> index c629f39..327a668 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -822,7 +822,15 @@ brwCreateContext(gl_api api,
> _mesa_meta_init(ctx);
>
> brw_process_driconf_options(brw);
> - brw_process_intel_debug_variable(brw);
> +
> + if (INTEL_DEBUG & DEBUG_BUFMGR)
> + dri_bufmgr_set_debug(brw->bufmgr, true);
This should be done at screen creation time. brw->bufmgr is just a
shadow copy of intelScreen->bufmgr; there is only one bufmgr for the
whole process.
> +
> + if (INTEL_DEBUG & DEBUG_PERF)
> + brw->perf_debug = true;
> +
> + if (INTEL_DEBUG & DEBUG_AUB)
> + drm_intel_bufmgr_gem_set_aub_dump(brw->bufmgr, true);
Ditto for aub dumping.
Perhaps just pass the screen into brw_process_intel_debug_variable
instead of devinfo? Or also pass the bufmgr?
>
> if (brw->gen >= 8 && !(INTEL_DEBUG & DEBUG_VEC4VS))
>brw->scalar_vs = true;
> diff --git a/src/mesa/drivers/dri/i965/intel_debug.c
> b/src/mesa/drivers/dri/i965/intel_debug.c
> index 53f575a..0f4e556 100644
> --- a/src/mesa/drivers/dri/i965/intel_debug.c
> +++ b/src/mesa/drivers/dri/i965/intel_debug.c
> @@ -88,25 +88,16 @@ intel_debug_flag_for_shader_stage(gl_shader_stage stage)
> }
>
> void
> -brw_process_intel_debug_variable(struct brw_context *brw)
> +brw_process_intel_debug_variable(const struct brw_device_info *devinfo)
> {
> uint64_t intel_debug = driParseDebugString(getenv("INTEL_DEBUG"),
> debug_control);
> (void) p_atomic_cmpxchg(&INTEL_DEBUG, 0, intel_debug);
>
> - if (INTEL_DEBUG & DEBUG_BUFMGR)
> - dri_bufmgr_set_debug(brw->bufmgr, true);
> -
> - if ((INTEL_DEBUG & DEBUG_SHADER_TIME) && brw->gen < 7) {
> + if ((INTEL_DEBUG & DEBUG_SHADER_TIME) && devinfo->gen < 7) {
>fprintf(stderr,
>"shader_time debugging requires gen7 (Ivybridge) or
> better.\n");
>INTEL_DEBUG &= ~DEBUG_SHADER_TIME;
> }
> -
> - if (INTEL_DEBUG & DEBUG_PERF)
> - brw->perf_debug = true;
> -
> - if (INTEL_DEBUG & DEBUG_AUB)
> - drm_intel_bufmgr_gem_set_aub_dump(brw->bufmgr, true);
> }
>
> /**
> diff --git a/src/mesa/drivers/dri/i965/intel_debug.h
> b/src/mesa/drivers/dri/i965/intel_debug.h
> index f754be2..96212df 100644
> --- a/src/mesa/drivers/dri/i965/intel_debug.h
> +++ b/src/mesa/drivers/dri/i965/intel_debug.h
> @@ -114,8 +114,8 @@ extern uint64_t INTEL_DEBUG;
>
> extern uint64_t intel_debug_flag_for_shader_stage(gl_shader_stage stage);
>
> -struct brw_context;
> +struct brw_device_info;
>
> -extern void brw_process_intel_debug_variable(struct brw_context *brw);
> +extern void brw_process_intel_debug_variable(const struct brw_device_info *);
>
> extern bool brw_env_var_as_boolean(const char *var_name, bool default_value);
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
> b/src/mesa/drivers/dri/i965/intel_screen.c
> index 896a125..38475b9 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -1372,6 +1372,8 @@ __DRIconfig **intelInitScreen2(__DRIscreen *psp)
> if (!intelScreen->devinfo)
>return false;
>
> + brw_process_intel_debug_variable(intelScreen->devinfo);
> +
> intelScreen->hw_must_use_separate_stencil = intelScreen->devinfo->gen >=
> 7;
>
> intelScreen->hw_has_swizzling = intel_detect_swizzling(intelScreen);
>
signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/16] i965: Add compiler options to brw_compiler
On Monday, June 22, 2015 06:07:29 PM Jason Ekstrand wrote:
> This creates the options at screen cration time and then we just copy them
> into the context at context creation time. We also move is_scalar to the
> brw_compiler structure.
>
> We also end up manually setting some values that the core would have set by
> default for us. Fortunately, there are only two non-zero shader compiler
> option defaults that we aren't overriding anyway so this isn't a big deal.
> ---
> src/mesa/drivers/dri/i965/brw_context.c | 46 ++
> src/mesa/drivers/dri/i965/brw_context.h | 1 -
> src/mesa/drivers/dri/i965/brw_shader.cpp | 49
> +++-
> src/mesa/drivers/dri/i965/brw_shader.h | 3 ++
> src/mesa/drivers/dri/i965/brw_vec4.cpp | 2 +-
> src/mesa/drivers/dri/i965/intel_screen.c | 1 +
> 6 files changed, 56 insertions(+), 46 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c
> b/src/mesa/drivers/dri/i965/brw_context.c
> index 327a668..33cdbd2 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -50,6 +50,7 @@
>
> #include "brw_context.h"
> #include "brw_defines.h"
> +#include "brw_shader.h"
> #include "brw_draw.h"
> #include "brw_state.h"
>
> @@ -68,8 +69,6 @@
> #include "tnl/t_pipeline.h"
> #include "util/ralloc.h"
>
> -#include "glsl/nir/nir.h"
> -
> /***
> * Mesa's Driver Functions
> ***/
> @@ -558,48 +557,12 @@ brw_initialize_context_constants(struct brw_context
> *brw)
>ctx->Const.Program[MESA_SHADER_FRAGMENT].MaxInputComponents = 128;
> }
>
> - static const nir_shader_compiler_options nir_options = {
> - .native_integers = true,
> - /* In order to help allow for better CSE at the NIR level we tell NIR
> - * to split all ffma instructions during opt_algebraic and we then
> - * re-combine them as a later step.
> - */
> - .lower_ffma = true,
> - .lower_sub = true,
> - };
> -
> /* We want the GLSL compiler to emit code that uses condition codes */
> for (int i = 0; i < MESA_SHADER_STAGES; i++) {
> - ctx->Const.ShaderCompilerOptions[i].MaxIfDepth = brw->gen < 6 ? 16 :
> UINT_MAX;
> - ctx->Const.ShaderCompilerOptions[i].EmitCondCodes = true;
> - ctx->Const.ShaderCompilerOptions[i].EmitNoNoise = true;
> - ctx->Const.ShaderCompilerOptions[i].EmitNoMainReturn = true;
> - ctx->Const.ShaderCompilerOptions[i].EmitNoIndirectInput = true;
> - ctx->Const.ShaderCompilerOptions[i].EmitNoIndirectOutput =
> - (i == MESA_SHADER_FRAGMENT);
> - ctx->Const.ShaderCompilerOptions[i].EmitNoIndirectTemp =
> - (i == MESA_SHADER_FRAGMENT);
> - ctx->Const.ShaderCompilerOptions[i].EmitNoIndirectUniform = false;
> - ctx->Const.ShaderCompilerOptions[i].LowerClipDistance = true;
> + ctx->Const.ShaderCompilerOptions[i] =
> + brw->intelScreen->compiler->glsl_compiler_options[i];
> }
>
> - ctx->Const.ShaderCompilerOptions[MESA_SHADER_VERTEX].OptimizeForAOS =
> true;
> - ctx->Const.ShaderCompilerOptions[MESA_SHADER_GEOMETRY].OptimizeForAOS =
> true;
> -
> - if (brw->scalar_vs) {
> - /* If we're using the scalar backend for vertex shaders, we need to
> - * configure these accordingly.
> - */
> -
> ctx->Const.ShaderCompilerOptions[MESA_SHADER_VERTEX].EmitNoIndirectOutput =
> true;
> -
> ctx->Const.ShaderCompilerOptions[MESA_SHADER_VERTEX].EmitNoIndirectTemp =
> true;
> - ctx->Const.ShaderCompilerOptions[MESA_SHADER_VERTEX].OptimizeForAOS =
> false;
> -
> - ctx->Const.ShaderCompilerOptions[MESA_SHADER_VERTEX].NirOptions =
> &nir_options;
> - }
> -
> - ctx->Const.ShaderCompilerOptions[MESA_SHADER_FRAGMENT].NirOptions =
> &nir_options;
> - ctx->Const.ShaderCompilerOptions[MESA_SHADER_COMPUTE].NirOptions =
> &nir_options;
> -
> /* ARB_viewport_array */
> if (brw->gen >= 6 && ctx->API == API_OPENGL_CORE) {
>ctx->Const.MaxViewports = GEN6_NUM_VIEWPORTS;
> @@ -832,9 +795,6 @@ brwCreateContext(gl_api api,
> if (INTEL_DEBUG & DEBUG_AUB)
>drm_intel_bufmgr_gem_set_aub_dump(brw->bufmgr, true);
>
> - if (brw->gen >= 8 && !(INTEL_DEBUG & DEBUG_VEC4VS))
> - brw->scalar_vs = true;
> -
> brw_initialize_context_constants(brw);
>
> ctx->Const.ResetStrategy = notify_reset
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h
> b/src/mesa/drivers/dri/i965/brw_context.h
> index 58119ee..d8fcfff 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -1137,7 +1137,6 @@ struct brw_context
> bool has_pln;
> bool no_simd8;
> bool use_rep_send;
> - bool scalar_vs;
>
> /**
> * Some versions of Gen hardware don't do centroid interpolation correctly
> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp
> b/src/mesa/drivers/dri/i965/b
[Mesa-dev] [Bug 90797] [ALL bisected] Mesa change cause performance case manhattan fail.
https://bugs.freedesktop.org/show_bug.cgi?id=90797 Tapani Pälli changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED Assignee|mesa-dev@lists.freedesktop. |lem...@gmail.com |org | --- Comment #10 from Tapani Pälli --- Fixed in master: commit 7d88ab42b9dda825feddbae774a2a48ddf3cbec2 Author: Tapani Pälli Date: Tue Jun 16 13:46:47 2015 +0300 mesa: set override_version per api version override Before 9b5e92f get_gl_override was called only once, but now it is called for multiple APIs (GLES2, GL), version needs to be set always. -- 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 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] i965: Split out gen8 push constant state upload
On Wed, Jun 3, 2015 at 9:35 PM, Ben Widawsky
wrote:
> While implementing the workaround in the previous patch I noticed things were
> starting to get a bit messy. Since gen8 works differently enough from gen7, I
> thought splitting it out with be good.
>
> While here, get rid of gen8 MOCS which does nothing and was in the wrong place
> anyway.
>
> This patch is totally optional. I'd be willing to just always use buffer #2 on
> gen8+. Pre-HSW this wasn't allowed, but it looks like it's okay for GEN8 too.
>
> Signed-off-by: Ben Widawsky
> ---
> src/mesa/drivers/dri/i965/brw_state.h | 6 +--
> src/mesa/drivers/dri/i965/gen6_gs_state.c | 2 +-
> src/mesa/drivers/dri/i965/gen6_vs_state.c | 3 +-
> src/mesa/drivers/dri/i965/gen6_wm_state.c | 3 +-
> src/mesa/drivers/dri/i965/gen7_vs_state.c | 82
> ---
> 5 files changed, 59 insertions(+), 37 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_state.h
> b/src/mesa/drivers/dri/i965/brw_state.h
> index 987672f..f45459d 100644
> --- a/src/mesa/drivers/dri/i965/brw_state.h
> +++ b/src/mesa/drivers/dri/i965/brw_state.h
> @@ -368,9 +368,9 @@ brw_upload_pull_constants(struct brw_context *brw,
>
> /* gen7_vs_state.c */
> void
> -gen7_upload_constant_state(struct brw_context *brw,
> - const struct brw_stage_state *stage_state,
> - bool active, unsigned opcode);
> +brw_upload_constant_state(struct brw_context *brw,
> + const struct brw_stage_state *stage_state,
> + bool active, unsigned opcode);
>
> #ifdef __cplusplus
> }
> diff --git a/src/mesa/drivers/dri/i965/gen6_gs_state.c
> b/src/mesa/drivers/dri/i965/gen6_gs_state.c
> index eb4c586..19568b0 100644
> --- a/src/mesa/drivers/dri/i965/gen6_gs_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_gs_state.c
> @@ -48,7 +48,7 @@ gen6_upload_gs_push_constants(struct brw_context *brw)
> }
>
> if (brw->gen >= 7)
> - gen7_upload_constant_state(brw, stage_state, gp, _3DSTATE_CONSTANT_GS);
> + brw_upload_constant_state(brw, stage_state, gp, _3DSTATE_CONSTANT_GS);
> }
>
> const struct brw_tracked_state gen6_gs_push_constants = {
> diff --git a/src/mesa/drivers/dri/i965/gen6_vs_state.c
> b/src/mesa/drivers/dri/i965/gen6_vs_state.c
> index 35d10ef..c33607d 100644
> --- a/src/mesa/drivers/dri/i965/gen6_vs_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_vs_state.c
> @@ -140,8 +140,7 @@ gen6_upload_vs_push_constants(struct brw_context *brw)
>if (brw->gen == 7 && !brw->is_haswell && !brw->is_baytrail)
> gen7_emit_vs_workaround_flush(brw);
>
> - gen7_upload_constant_state(brw, stage_state, true /* active */,
> - _3DSTATE_CONSTANT_VS);
> + brw_upload_constant_state(brw, stage_state, true,
> _3DSTATE_CONSTANT_VS);
> }
> }
>
> diff --git a/src/mesa/drivers/dri/i965/gen6_wm_state.c
> b/src/mesa/drivers/dri/i965/gen6_wm_state.c
> index 7081eb7..1cfd1ad 100644
> --- a/src/mesa/drivers/dri/i965/gen6_wm_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_wm_state.c
> @@ -49,8 +49,7 @@ gen6_upload_wm_push_constants(struct brw_context *brw)
>stage_state, AUB_TRACE_WM_CONSTANTS);
>
> if (brw->gen >= 7) {
> - gen7_upload_constant_state(brw, &brw->wm.base, true,
> - _3DSTATE_CONSTANT_PS);
> + brw_upload_constant_state(brw, &brw->wm.base, true,
> _3DSTATE_CONSTANT_PS);
> }
> }
>
> diff --git a/src/mesa/drivers/dri/i965/gen7_vs_state.c
> b/src/mesa/drivers/dri/i965/gen7_vs_state.c
> index 4b17d06..091df53 100644
> --- a/src/mesa/drivers/dri/i965/gen7_vs_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_vs_state.c
> @@ -29,20 +29,16 @@
> #include "program/prog_statevars.h"
> #include "intel_batchbuffer.h"
>
> -
> -void
> -gen7_upload_constant_state(struct brw_context *brw,
> +static void
> +gen8_upload_constant_state(struct brw_context *brw,
> const struct brw_stage_state *stage_state,
> bool active, unsigned opcode)
> {
> - uint32_t mocs = brw->gen < 8 ? GEN7_MOCS_L3 : 0;
>
> - /* Disable if the shader stage is inactive or there are no push
> constants. */
> - active = active && stage_state->push_const_size != 0;
> + /* MOCS is optional for GEN9, but it isn't allowed for GEN8 */
>
> - int dwords = brw->gen >= 8 ? 11 : 7;
> - BEGIN_BATCH(dwords);
> - OUT_BATCH(opcode << 16 | (dwords - 2));
> + BEGIN_BATCH(11);
> + OUT_BATCH(opcode << 16 | (11 - 2));
>
> /* Workaround for SKL+ (we use option #2 until we have a need for more
> * constant buffers). This comes from the documentation for
> 3DSTATE_CONSTANT_*
> @@ -57,42 +53,40 @@ gen7_upload_constant_state(struct brw_context *brw,
> */
> if (brw->gen >= 9 && active) {
>OUT_BATCH(0);
> - OUT_BATCH(stage_state->push_const_size);
> + OUT_BATCH(stage_state->push_const_size); /* buffer 3, 2 *
Re: [Mesa-dev] [PATCH 00/16] i965: Finish removing brw_context from the compiler
On Monday, June 22, 2015 06:07:20 PM Jason Ekstrand wrote: > I started working on this project some time ago to remove brw_context from > the backend compiler. I got a bunch of refactoring done but eventualy got > stuck up on shader_time and some debug logging stuff. I've finally gotten > around to finishing it and here it is. > > Jason Ekstrand (15): > i965: Replace some instances of brw->gen with devinfo->gen > i965: Plumb compiler debug logging through a function pointer in > brw_compiler > i965: Remove the dependance on brw_context from the generators > i965: Move INTEL_DEBUG variable parsing to screen creation time > i965/fs: Make no16 non-variadic > i965/fs: Do the no16 perf logging directly in fs_visitor::no16() > i965/fs: Plumb compiler debug logging through brw_compiler > i965: Add compiler options to brw_compiler > i965: Use a single index per shader for shader_time. > i965: Pull calls to get_shader_time_index out of the visitor > i965/fs: Add a do_rep_send flag to run_fs > i965/vs: Pass the current set of clip planes through run() and > run_vs() > i965/vec4: Turn some _mesa_problem calls into asserts > i965/vec4_vs: Add an explicit use_legacy_snorm_formula flag > i965: Remove the brw_context from the visitors > > Kenneth Graunke (1): > mesa: Add a va_args variant of _mesa_gl_debug(). I requested a few small changes. With those fixed, For the series (minus my patch), Reviewed-by: Kenneth Graunke signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] tgsi_to_nir: Fix translation of TXF on MSAA targets.
Noticed while trying to add GL_ARB_texture_multisample support to vc4.
---
src/gallium/auxiliary/nir/tgsi_to_nir.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/src/gallium/auxiliary/nir/tgsi_to_nir.c
b/src/gallium/auxiliary/nir/tgsi_to_nir.c
index 061f39a..065bbf0 100644
--- a/src/gallium/auxiliary/nir/tgsi_to_nir.c
+++ b/src/gallium/auxiliary/nir/tgsi_to_nir.c
@@ -1078,7 +1078,12 @@ ttn_tex(struct ttn_compile *c, nir_alu_dest dest,
nir_ssa_def **src)
samp = 2;
break;
case TGSI_OPCODE_TXF:
- op = nir_texop_txf;
+ if (tgsi_inst->Texture.Texture == TGSI_TEXTURE_2D_MSAA ||
+ tgsi_inst->Texture.Texture == TGSI_TEXTURE_2D_ARRAY_MSAA) {
+ op = nir_texop_txf_ms;
+ } else {
+ op = nir_texop_txf;
+ }
num_srcs = 2;
break;
case TGSI_OPCODE_TXD:
@@ -1178,7 +1183,10 @@ ttn_tex(struct ttn_compile *c, nir_alu_dest dest,
nir_ssa_def **src)
if (tgsi_inst->Instruction.Opcode == TGSI_OPCODE_TXF) {
instr->src[src_number].src = nir_src_for_ssa(ttn_channel(b, src[0], W));
- instr->src[src_number].src_type = nir_tex_src_lod;
+ if (op == nir_texop_txf_ms)
+ instr->src[src_number].src_type = nir_tex_src_ms_index;
+ else
+ instr->src[src_number].src_type = nir_tex_src_lod;
src_number++;
}
--
2.1.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 07/17] i965/fs: Move offset() and half() to the fs_builder
On Tue, Jun 23, 2015 at 9:22 AM, Francisco Jerez wrote:
> Jason Ekstrand writes:
>
>> We want to move these into the builder so that they know the current
>> builder's dispatch width. This will be needed by a later commit.
>
> I very much like the idea of this series, but, why do you need to move
> these register manipulators into the builder? The builder is an object
> you can use to:
> - Manipulate and query parameters affecting code generation.
> - Create instructions into the program (::emit and friends).
> - Allocate virtual registers from the program (::vgrf and friends).
>
> offset() and half() logically perform an action on a given register
> object (or rather, compute a function of a given register object), not
> on a builder object, the builder is only required as an auxiliary
> parameter -- Any reason you didn't just pass it as a third parameter?
What's required as a third parameter is the current execution size. I
could have passed that directly, but I figured that, especially for
half(), it would get messed up. I could pass the builder in but I
don't see a whole lot of difference between that and what I'm doing
right now. As is, it's not entirely obvious whether you should call
half(reg) on the half-width or full-width builder. I'm not 100% sure
what to do about that.
> As offset() and half() don't require access to any private details of
> the builder, that would actually improve encapsulation, and would avoid
> the dubious overloading of fs_builder::half() with two methods with
> completely different semantics.
Yeah, I don't really like that either. I just couldn't come up with
anything better at the time.
Suggestions are very much welcome. But I would like to settle on
whatever we do fairly quickly so as to limit the amount of
refactoring.
--Jason
> Thanks.
>
>> ---
>> src/mesa/drivers/dri/i965/brw_fs.cpp | 52 ++
>> src/mesa/drivers/dri/i965/brw_fs_builder.h | 46 +
>> src/mesa/drivers/dri/i965/brw_fs_cse.cpp | 2 +-
>> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 60 +--
>> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 149
>> ++-
>> src/mesa/drivers/dri/i965/brw_ir_fs.h| 51 -
>> 6 files changed, 182 insertions(+), 178 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> index 4f98d63..c13ac7d 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> @@ -267,7 +267,7 @@ fs_visitor::VARYING_PULL_CONSTANT_LOAD(const fs_builder
>> &bld,
>> inst->mlen = 1 + dispatch_width / 8;
>> }
>>
>> - bld.MOV(dst, offset(vec4_result, (const_offset & 3) * scale));
>> + bld.MOV(dst, bld.offset(vec4_result, (const_offset & 3) * scale));
>> }
>>
>> /**
>> @@ -361,7 +361,12 @@ fs_inst::is_copy_payload(const brw::simple_allocator
>> &grf_alloc) const
>>reg.width = this->src[i].width;
>>if (!this->src[i].equals(reg))
>> return false;
>> - reg = ::offset(reg, 1);
>> +
>> + if (i < this->header_size) {
>> + reg.reg_offset += 1;
>> + } else {
>> + reg.reg_offset += this->exec_size / 8;
>> + }
>> }
>>
>> return true;
>> @@ -963,7 +968,7 @@ fs_visitor::emit_fragcoord_interpolation(bool
>> pixel_center_integer,
>> } else {
>>bld.ADD(wpos, this->pixel_x, fs_reg(0.5f));
>> }
>> - wpos = offset(wpos, 1);
>> + wpos = bld.offset(wpos, 1);
>>
>> /* gl_FragCoord.y */
>> if (!flip && pixel_center_integer) {
>> @@ -979,7 +984,7 @@ fs_visitor::emit_fragcoord_interpolation(bool
>> pixel_center_integer,
>>
>>bld.ADD(wpos, pixel_y, fs_reg(offset));
>> }
>> - wpos = offset(wpos, 1);
>> + wpos = bld.offset(wpos, 1);
>>
>> /* gl_FragCoord.z */
>> if (devinfo->gen >= 6) {
>> @@ -989,7 +994,7 @@ fs_visitor::emit_fragcoord_interpolation(bool
>> pixel_center_integer,
>> this->delta_xy[BRW_WM_PERSPECTIVE_PIXEL_BARYCENTRIC],
>> interp_reg(VARYING_SLOT_POS, 2));
>> }
>> - wpos = offset(wpos, 1);
>> + wpos = bld.offset(wpos, 1);
>>
>> /* gl_FragCoord.w: Already set up in emit_interpolation */
>> bld.MOV(wpos, this->wpos_w);
>> @@ -1072,7 +1077,7 @@ fs_visitor::emit_general_interpolation(fs_reg attr,
>> const char *name,
>> /* If there's no incoming setup data for this slot, don't
>>* emit interpolation for it.
>>*/
>> - attr = offset(attr, type->vector_elements);
>> + attr = bld.offset(attr, type->vector_elements);
>> location++;
>> continue;
>>}
>> @@ -1087,7 +1092,7 @@ fs_visitor::emit_general_interpolation(fs_reg attr,
>> const char *name,
>> interp = suboffset(interp, 3);
>> interp.type = attr.type;
>> bld.emit(FS_OPCODE_CINTERP, attr, fs_reg(interp));
>> -attr = offset(attr, 1);
>> +attr = bld.of
Re: [Mesa-dev] [PATCH 07/17] i965/fs: Move offset() and half() to the fs_builder
On Tue, Jun 23, 2015 at 1:39 AM, Pohjolainen, Topi
wrote:
> On Thu, Jun 18, 2015 at 05:51:36PM -0700, Jason Ekstrand wrote:
>> We want to move these into the builder so that they know the current
>> builder's dispatch width. This will be needed by a later commit.
>> ---
>> src/mesa/drivers/dri/i965/brw_fs.cpp | 52 ++
>> src/mesa/drivers/dri/i965/brw_fs_builder.h | 46 +
>> src/mesa/drivers/dri/i965/brw_fs_cse.cpp | 2 +-
>> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 60 +--
>> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 149
>> ++-
>> src/mesa/drivers/dri/i965/brw_ir_fs.h| 51 -
>> 6 files changed, 182 insertions(+), 178 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> index 4f98d63..c13ac7d 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> @@ -267,7 +267,7 @@ fs_visitor::VARYING_PULL_CONSTANT_LOAD(const fs_builder
>> &bld,
>> inst->mlen = 1 + dispatch_width / 8;
>> }
>>
>> - bld.MOV(dst, offset(vec4_result, (const_offset & 3) * scale));
>> + bld.MOV(dst, bld.offset(vec4_result, (const_offset & 3) * scale));
>> }
>>
>> /**
>> @@ -361,7 +361,12 @@ fs_inst::is_copy_payload(const brw::simple_allocator
>> &grf_alloc) const
>>reg.width = this->src[i].width;
>>if (!this->src[i].equals(reg))
>> return false;
>> - reg = ::offset(reg, 1);
>> +
>> + if (i < this->header_size) {
>> + reg.reg_offset += 1;
>> + } else {
>> + reg.reg_offset += this->exec_size / 8;
>> + }
>
> The latter branch is new functionality, isn't it? There is no consideration
> for header_size in the offset() utility.
Not quite. We don't have a builder in this context, so I had to
mangle the reg_offset itself. I'll freely admit that's kind of ugly.
This might be a good argument for Curro's suggestion of just adding a
width or maybe a pair of widths to the half() function.
The reason for the if statement is that, if it's a header, it deals in
actual physical registers while, if it's not a header, it deals in
something relative to the width. This was all magically handled by
offset() before because the header registers had a width of 8 and the
others had a width of exec_size.
>> }
>>
>> return true;
>> @@ -963,7 +968,7 @@ fs_visitor::emit_fragcoord_interpolation(bool
>> pixel_center_integer,
>> } else {
>>bld.ADD(wpos, this->pixel_x, fs_reg(0.5f));
>> }
>> - wpos = offset(wpos, 1);
>> + wpos = bld.offset(wpos, 1);
>>
>> /* gl_FragCoord.y */
>> if (!flip && pixel_center_integer) {
>> @@ -979,7 +984,7 @@ fs_visitor::emit_fragcoord_interpolation(bool
>> pixel_center_integer,
>>
>>bld.ADD(wpos, pixel_y, fs_reg(offset));
>> }
>> - wpos = offset(wpos, 1);
>> + wpos = bld.offset(wpos, 1);
>>
>> /* gl_FragCoord.z */
>> if (devinfo->gen >= 6) {
>> @@ -989,7 +994,7 @@ fs_visitor::emit_fragcoord_interpolation(bool
>> pixel_center_integer,
>> this->delta_xy[BRW_WM_PERSPECTIVE_PIXEL_BARYCENTRIC],
>> interp_reg(VARYING_SLOT_POS, 2));
>> }
>> - wpos = offset(wpos, 1);
>> + wpos = bld.offset(wpos, 1);
>>
>> /* gl_FragCoord.w: Already set up in emit_interpolation */
>> bld.MOV(wpos, this->wpos_w);
>> @@ -1072,7 +1077,7 @@ fs_visitor::emit_general_interpolation(fs_reg attr,
>> const char *name,
>> /* If there's no incoming setup data for this slot, don't
>>* emit interpolation for it.
>>*/
>> - attr = offset(attr, type->vector_elements);
>> + attr = bld.offset(attr, type->vector_elements);
>> location++;
>> continue;
>>}
>> @@ -1087,7 +1092,7 @@ fs_visitor::emit_general_interpolation(fs_reg attr,
>> const char *name,
>> interp = suboffset(interp, 3);
>> interp.type = attr.type;
>> bld.emit(FS_OPCODE_CINTERP, attr, fs_reg(interp));
>> -attr = offset(attr, 1);
>> +attr = bld.offset(attr, 1);
>> }
>>} else {
>> /* Smooth/noperspective interpolation case. */
>> @@ -1125,7 +1130,7 @@ fs_visitor::emit_general_interpolation(fs_reg attr,
>> const char *name,
>> if (devinfo->gen < 6 && interpolation_mode ==
>> INTERP_QUALIFIER_SMOOTH) {
>>bld.MUL(attr, attr, this->pixel_w);
>> }
>> -attr = offset(attr, 1);
>> +attr = bld.offset(attr, 1);
>> }
>>
>>}
>> @@ -1227,19 +1232,19 @@ fs_visitor::emit_samplepos_setup()
>> if (dispatch_width == 8) {
>>abld.MOV(int_sample_x, fs_reg(sample_pos_reg));
>> } else {
>> - abld.half(0).MOV(half(int_sample_x, 0), fs_reg(sample_pos_reg));
>> - abld.half(1).MOV(half(int_sample_x, 1),
>> + abld.half(0).MOV(abld.half(int_sample_x, 0), fs_reg(sample_p
[Mesa-dev] [Bug 91077] dri2_glx.c:1186: undefined reference to `loader_open_device'
https://bugs.freedesktop.org/show_bug.cgi?id=91077 Bug ID: 91077 Summary: dri2_glx.c:1186: undefined reference to `loader_open_device' Product: Mesa Version: git Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Keywords: bisected, regression Severity: normal Priority: medium Component: Mesa core Assignee: mesa-dev@lists.freedesktop.org Reporter: v...@freedesktop.org QA Contact: mesa-dev@lists.freedesktop.org CC: der...@osg.samsung.com, emil.l.veli...@gmail.com, k...@bitplanet.net mesa: 3fa9bb81ec8b21f472de32e08d0caf917239da08 (master 10.7.0-devel) 324ee9b391ea2db4b74709d30a131e79055bf071 is the first bad commit commit 324ee9b391ea2db4b74709d30a131e79055bf071 Author: Derek Foreman Date: Wed Jun 17 11:28:50 2015 -0500 glx: Use loader_open_device() helper We've moved the open with CLOEXEC idiom into a helper function, so call it instead of duplicating the code here. Signed-off-by: Derek Foreman Reviewed-by: Kristian Høgsberg Reviewed-by: Emil Velikov :04 04 57dcc9a36cb72627a1e91340f099355200434adb 180705d900106982a31274f4a068c8bc39526a7c Msrc bisect run success -- 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 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 91077] dri2_glx.c:1186: undefined reference to `loader_open_device'
https://bugs.freedesktop.org/show_bug.cgi?id=91077 --- Comment #1 from Derek Foreman --- What configure options are you using? Can you attach a build log? Do you have any local changes? Having a hard time figuring this out, I can't reproduce it here, and dri2_glx.c uses other functions from loader.c so there should at least be a pile of other undefined references. Can you do a make clean and try again? Perhaps you have some kind of timestamp issue that led to a partial rebuild... -- 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 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] tgsi_to_nir: Fix translation of TXF on MSAA targets.
Reviewed-by: Ilia Mirkin
On Tue, Jun 23, 2015 at 2:04 PM, Eric Anholt wrote:
> Noticed while trying to add GL_ARB_texture_multisample support to vc4.
> ---
> src/gallium/auxiliary/nir/tgsi_to_nir.c | 12 ++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/auxiliary/nir/tgsi_to_nir.c
> b/src/gallium/auxiliary/nir/tgsi_to_nir.c
> index 061f39a..065bbf0 100644
> --- a/src/gallium/auxiliary/nir/tgsi_to_nir.c
> +++ b/src/gallium/auxiliary/nir/tgsi_to_nir.c
> @@ -1078,7 +1078,12 @@ ttn_tex(struct ttn_compile *c, nir_alu_dest dest,
> nir_ssa_def **src)
>samp = 2;
>break;
> case TGSI_OPCODE_TXF:
> - op = nir_texop_txf;
> + if (tgsi_inst->Texture.Texture == TGSI_TEXTURE_2D_MSAA ||
> + tgsi_inst->Texture.Texture == TGSI_TEXTURE_2D_ARRAY_MSAA) {
> + op = nir_texop_txf_ms;
> + } else {
> + op = nir_texop_txf;
> + }
>num_srcs = 2;
>break;
> case TGSI_OPCODE_TXD:
> @@ -1178,7 +1183,10 @@ ttn_tex(struct ttn_compile *c, nir_alu_dest dest,
> nir_ssa_def **src)
>
> if (tgsi_inst->Instruction.Opcode == TGSI_OPCODE_TXF) {
>instr->src[src_number].src = nir_src_for_ssa(ttn_channel(b, src[0],
> W));
> - instr->src[src_number].src_type = nir_tex_src_lod;
> + if (op == nir_texop_txf_ms)
> + instr->src[src_number].src_type = nir_tex_src_ms_index;
> + else
> + instr->src[src_number].src_type = nir_tex_src_lod;
>src_number++;
> }
>
> --
> 2.1.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 5/5] i965/gen9: Allocate YF/YS tiled buffer objects
In case of I915_TILING_{X,Y} we need to pass tiling format to libdrm
using drm_intel_bo_alloc_tiled(). But, In case of YF/YS tiled buffers
libdrm need not know about the tiling format because these buffers
don't have hardware support to be tiled or detiled through a fenced
region. libdrm still need to know buffer alignment value for its use
in kernel when resolving the relocation.
Using drm_intel_bo_alloc_for_render() for YF/YS tiled buffers
satisfy both the above conditions.
V2: Delete min/max buffer size restrictions not valid for i965+.
Remove redundant align to tile size statements.
Remove some redundant code now when there are no min/max buffer size.
Signed-off-by: Anuj Phogat
Cc: Ben Widawsky
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 62 +--
1 file changed, 58 insertions(+), 4 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 80c52f2..5bcb094 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -558,6 +558,48 @@ intel_lower_compressed_format(struct brw_context *brw,
mesa_format format)
}
}
+/* This function computes Yf/Ys tiled bo size, alignment and pitch. */
+static uint64_t
+intel_get_yf_ys_bo_size(struct intel_mipmap_tree *mt, unsigned *alignment,
+uint64_t *pitch)
+{
+ const uint32_t bpp = mt->cpp * 8;
+ const uint32_t aspect_ratio = (bpp == 16 || bpp == 64) ? 2 : 1;
+ uint32_t tile_width, tile_height;
+ uint64_t stride, size, aligned_y;
+
+ assert(mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE);
+
+ *alignment = mt->tr_mode == INTEL_MIPTREE_TRMODE_YF ? 4096 : 64 * 1024;
+
+ switch (bpp) {
+ case 8:
+ tile_height = 64;
+ break;
+ case 16:
+ case 32:
+ tile_height = 32;
+ break;
+ case 64:
+ case 128:
+ tile_height = 16;
+ break;
+ default:
+ unreachable("not reached");
+ }
+
+ if (mt->tr_mode == INTEL_MIPTREE_TRMODE_YS)
+ tile_height *= 4;
+
+ aligned_y = ALIGN(mt->total_height, tile_height);
+ stride = mt->total_width * mt->cpp;
+ tile_width = tile_height * mt->cpp * aspect_ratio;
+ stride = ALIGN(stride, tile_width);
+ size = stride * aligned_y;
+
+ *pitch = stride;
+ return size;
+}
struct intel_mipmap_tree *
intel_miptree_create(struct brw_context *brw,
@@ -616,11 +658,23 @@ intel_miptree_create(struct brw_context *brw,
alloc_flags |= BO_ALLOC_FOR_RENDER;
unsigned long pitch;
- mt->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "miptree", total_width,
- total_height, mt->cpp, &mt->tiling,
- &pitch, alloc_flags);
mt->etc_format = etc_format;
- mt->pitch = pitch;
+
+ if (mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE) {
+ unsigned alignment = 0;
+ unsigned long size;
+ size = intel_get_yf_ys_bo_size(mt, &alignment, &pitch);
+ assert(size);
+ mt->bo = drm_intel_bo_alloc_for_render(brw->bufmgr, "miptree",
+ size, alignment);
+ mt->pitch = pitch;
+ } else {
+ mt->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "miptree",
+total_width, total_height, mt->cpp,
+&mt->tiling, &pitch,
+alloc_flags);
+ mt->pitch = pitch;
+ }
/* If the BO is too large to fit in the aperture, we need to use the
* BLT engine to support it. Prior to Sandybridge, the BLT paths can't
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 6/6] mesa/es3.1: Fix error code for glCreateShaderProgram
On Tue, Jun 23, 2015 at 5:23 AM, Marta Lofstedt
wrote:
> From: Marta Lofstedt
>
> According to the OpenGL ES standard, 7.3.
> For a call to glCreateShaderProgram with count < 0,
> a GL_INVALID_VALUE error should be generated.
>
> Signed-off-by: Marta Lofstedt
> ---
> src/mesa/main/shaderapi.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> index c783c69..266064d 100644
> --- a/src/mesa/main/shaderapi.c
> +++ b/src/mesa/main/shaderapi.c
> @@ -1890,6 +1890,15 @@ _mesa_create_shader_program(struct gl_context* ctx,
> GLboolean separate,
> const GLuint shader = create_shader(ctx, type);
> GLuint program = 0;
>
> + /*
> +* According to OpenGL ES 3.1 standard 7.3: GL_INVALID_VALUE
> +* should be generated, if count < 0.
> +*/
The format of spec citations is
/* Page 65 in section 7.3 Program Objects of the OpenGL ES 3.1 spec says:
*
* "An INVALID_VALUE error is generated if count is negative."
*
* "If an error is generated, zero is returned."
*/
> + if (_mesa_is_gles31(ctx) && count < 0) {
glCreateShaderProgramv comes from ARB_separate_shader_objects (merged
in GL 4.1), and in GL 4.3 the spec gained the new text cited above. I
think we should take that change as a clarification (a change that
should apply to previous versions retroactively) instead of an actual
change in behavior. As such, I think we should remove the
_mesa_is_gles31 check. I'd modify the citation to mention GL 4.3 as
well.
> + _mesa_error(ctx, GL_INVALID_VALUE, "glCreateShaderProgram (count <
> 0)");
Missing the "v" at the end of "glCreateShaderProgramv"
> + return program;
Just return literal 0 here to make it more clear.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 6/6] mesa/es3.1: Fix error code for glCreateShaderProgram
I should have also mentioned -- the commit titles need some improvement. "Fix error code" isn't very descriptive of the change, and the change isn't actually specific to es3.1. How about > mesa: Raise INVALID_VALUE from glCreateShaderProgramv if count < 0. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/6] mesa/es3.1 : Correct error code for defect texture target
On Tue, Jun 23, 2015 at 5:23 AM, Marta Lofstedt
wrote:
> From: Marta Lofstedt
>
> According to GLES 3.1 CTS test:
> ES31-CTS.texture_storage_multisample.
> APIGLGetTexLevelParameterifv.
> invalid_texture_target_rejected:
>
> GL_INVALID_ENUM should be generated when
> glGetTexLevelParameteriv is called with a defect
> texture target.
>
> Signed-off-by: Marta Lofstedt
> ---
> src/mesa/main/texobj.c | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/src/mesa/main/texobj.c b/src/mesa/main/texobj.c
> index c563f1e..c239deb 100644
> --- a/src/mesa/main/texobj.c
> +++ b/src/mesa/main/texobj.c
> @@ -222,6 +222,17 @@ _mesa_get_current_tex_object(struct gl_context *ctx,
> GLenum target)
Since this function is called internally from a lot of places, it's
not clear to me that it's appropriate to raise a GL error here, but
I'm not sure.
For instance, it's called by TexSubImage, and the spec says for TexSubImage...
An INVALID_VALUE error is generated by *TexSubImage* if target does
not match the command, as shown in table 8.15.
> return ctx->Extensions.ARB_texture_multisample
> ? ctx->Texture.ProxyTex[TEXTURE_2D_MULTISAMPLE_ARRAY_INDEX] :
> NULL;
>default:
> + if(_mesa_is_gles31(ctx))
Not specific to ES 3.1.
Commit message needs to be improved.
> + {
{ goes on the line with the if, like the rest of Mesa.
> +/*
> + * According to OpenGL ES 3.1 CTS:
> + *
> ES31-CTS.texture_storage_multisample.APIGLGetTexLevelParameterifv.
> + * invalid_value_argument_rejected
> + *
> es31cTextureStorageMultisampleGetTexLevelParameterifvTests.cpp:1277
> + * INVALID_ENUM should be reported for bad targets.
Like Erik says, need an actual spec citation.
> + */
> +_mesa_error(ctx, GL_INVALID_ENUM, "%s(target)", __func__);
> + }
> _mesa_problem(NULL, "bad target in _mesa_get_current_tex_object()");
> return NULL;
> }
> --
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/6] OpenGL ES 3.1 API checks and corner cases.
On Tue, Jun 23, 2015 at 5:23 AM, Marta Lofstedt wrote: > This is a series of patches that solves a couple of > API check and corner cases issues that the OpenGL ES 3.1 > CTS exploits. > > Marta Lofstedt (6): > mesa/es3.1: Do not allow zero size multisampled textures > mesa/es3.1: Correct error code for illegal internal formats > mesa/es3.1 : Correct error code for zero samples > mesa/es3.1 : Correct error code for defect texture target > mesa/es31: AtomicBufferBindings should be initialized to zero. > mesa/es3.1: Fix error code for glCreateShaderProgram > > src/mesa/main/bufferobj.c | 11 --- > src/mesa/main/shaderapi.c | 9 + > src/mesa/main/teximage.c | 25 + > src/mesa/main/texobj.c| 11 +++ > 4 files changed, 53 insertions(+), 3 deletions(-) > > -- I commented on #4 and #6, but the rest basically had the same problems: - the spec needs to cited, not the test suite - changes likely apply to desktop, don't raise errors just if(_mesa_gles_31(ctx)) - commit titles need to be more specific ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa : NULL check InfoLog
On Tue, Jun 23, 2015 at 4:03 AM, Marta Lofstedt wrote: > From: Marta Lofstedt > > When a program is compiled, but linking failed the > sh->InfoLog could be NULL. This is expoloited > by OpenGL ES 3.1 conformance tests. > > Signed-off-by: Marta Lofstedt > --- > src/mesa/main/shaderapi.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c > index a4296ad..c783c69 100644 > --- a/src/mesa/main/shaderapi.c > +++ b/src/mesa/main/shaderapi.c > @@ -1920,8 +1920,8 @@ _mesa_create_shader_program(struct gl_context* ctx, > GLboolean separate, > } > #endif > } > - > -ralloc_strcat(&shProg->InfoLog, sh->InfoLog); > + if (sh->InfoLog) > + ralloc_strcat(&shProg->InfoLog, sh->InfoLog); >} > >delete_shader(ctx, shader); > -- > 1.9.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev Reviewed-by: Anuj Phogat ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa : NULL check InfoLog
On Tue, Jun 23, 2015 at 4:03 AM, Marta Lofstedt wrote: > From: Marta Lofstedt > > When a program is compiled, but linking failed the > sh->InfoLog could be NULL. This is expoloited > by OpenGL ES 3.1 conformance tests. > > Signed-off-by: Marta Lofstedt > --- > src/mesa/main/shaderapi.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c > index a4296ad..c783c69 100644 > --- a/src/mesa/main/shaderapi.c > +++ b/src/mesa/main/shaderapi.c > @@ -1920,8 +1920,8 @@ _mesa_create_shader_program(struct gl_context* ctx, > GLboolean separate, > } > #endif > } > - > -ralloc_strcat(&shProg->InfoLog, sh->InfoLog); > + if (sh->InfoLog) > + ralloc_strcat(&shProg->InfoLog, sh->InfoLog); Wrong indentation on this line. Surely just not writing to the info log isn't the right fix? If it's null, shouldn't we instead ralloc_strdup() the string? ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl/es3.1: Fix up GL_ARB_compute_shader for GLSL ES 3.1
I don't really think the "/es3.1" in the commit summary adds anything. With that removed: Reviewed-by: Matt Turner ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl/es3.1: Fix up GL_ARB_compute_shader for GLSL ES 3.1
On Tue, Jun 23, 2015 at 7:20 AM, Marta Lofstedt
wrote:
> From: Marta Lofstedt
>
> GL_ARB_compute_shader is limited for GLSL version 430.
> This enables for GLSL ES version 310.
>
> Signed-off-by: Marta Lofstedt
> ---
> src/glsl/builtin_variables.cpp | 2 +-
> src/glsl/glsl_parser.yy| 3 +--
> src/glsl/glsl_parser_extras.h | 5 +
> 3 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/src/glsl/builtin_variables.cpp b/src/glsl/builtin_variables.cpp
> index a765d35..2681981 100644
> --- a/src/glsl/builtin_variables.cpp
> +++ b/src/glsl/builtin_variables.cpp
> @@ -695,7 +695,7 @@ builtin_variable_generator::generate_constants()
>}
> }
>
> - if (state->is_version(430, 0) || state->ARB_compute_shader_enable) {
> + if (state->has_compute_shader()) {
>add_const("gl_MaxComputeAtomicCounterBuffers",
> MAX_COMPUTE_ATOMIC_COUNTER_BUFFERS);
>add_const("gl_MaxComputeAtomicCounters", MAX_COMPUTE_ATOMIC_COUNTERS);
>add_const("gl_MaxComputeImageUniforms", MAX_COMPUTE_IMAGE_UNIFORMS);
> diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
> index 3ce9e10..757e548 100644
> --- a/src/glsl/glsl_parser.yy
> +++ b/src/glsl/glsl_parser.yy
> @@ -1483,8 +1483,7 @@ layout_qualifier_id:
> "invalid %s of %d specified",
> local_size_qualifiers[i], $3);
> YYERROR;
> -} else if (!state->is_version(430, 0) &&
> - !state->ARB_compute_shader_enable) {
> +} else if (!state->has_compute_shader()) {
> _mesa_glsl_error(& @3, state,
> "%s qualifier requires GLSL 4.30 or "
> "ARB_compute_shader",
Should this string mention GLSL ES 3.10 as well? Not sure what's done elsewhere.
> diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
> index a231d96..74838d3 100644
> --- a/src/glsl/glsl_parser_extras.h
> +++ b/src/glsl/glsl_parser_extras.h
> @@ -231,6 +231,11 @@ struct _mesa_glsl_parse_state {
>return ARB_gpu_shader_fp64_enable || is_version(400, 0);
> }
>
> + bool has_compute_shader() const
> + {
> + return ARB_compute_shader_enable || is_version(430, 310);
> + }
> +
> void process_version_directive(YYLTYPE *locp, int version,
>const char *ident);
>
> --
> 1.9.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] Fix strict-aliasing violations in GLSL shader list implementation
Hi - I'm new here.
I've recently started poking the Mesa codebase for little reason other
than personal interest. In the "help wanted" section of the website it
mentions aliasing violations as a target for newcomers to fix, so with
that in mind I've attached a patch (against git head) which resolves a
few of them, by targeting the linked list implementation (list.h) used
in the GLSL compiler/optimizers. This change slightly increases the
storage requirements for a list (adds one word) but resolves the blatant
aliasing violation that was caused by the trick used to conserve that
word in the first place.
(I toyed with another approach - using a single sentinel node for both
the head and tail of a list - but this was much more invasive, and meant
that you could no longer check whether a particular node was a sentinel
node unless you had a reference to the list, so I gave up and went with
this simpler approach).
The most essential change is in the 'exec_list' structure. Three fields
'head', 'tail' and 'tail_pred' are removed, and two separate sentinel
nodes are inserted in their place. The old 'head' is replaced by
'head_sentinel.next', 'tail_pred' by 'tail_sentinel.prev', and tail
(always NULL) by 'head_sentinel.prev' and 'tail_sentinel.next' (both
always NULL).
With this patch, I can build a working (though perhaps not 100%
bug-free) Mesa without using -fno-strict-aliasing during compilation.
Before the patch, applications using Mesa would hang at runtime.
I don't really know the process you have in place so if I need to do
anything else to get this patch into the codebase please let me know.
Any comments and criticisms welcome.
Davin
diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
index 92e26bf..f1c2ff5 100644
--- a/src/glsl/ast_function.cpp
+++ b/src/glsl/ast_function.cpp
@@ -152,8 +152,8 @@ verify_parameter_modes(_mesa_glsl_parse_state *state,
exec_list &actual_ir_parameters,
exec_list &actual_ast_parameters)
{
- exec_node *actual_ir_node = actual_ir_parameters.head;
- exec_node *actual_ast_node = actual_ast_parameters.head;
+ exec_node *actual_ir_node = actual_ir_parameters.head_sentinel.next;
+ exec_node *actual_ast_node = actual_ast_parameters.head_sentinel.next;
foreach_in_list(const ir_variable, formal, &sig->parameters) {
/* The lists must be the same length. */
@@ -964,7 +964,7 @@ constant_record_constructor(const glsl_type *constructor_type,
bool
single_scalar_parameter(exec_list *parameters)
{
- const ir_rvalue *const p = (ir_rvalue *) parameters->head;
+ const ir_rvalue *const p = (ir_rvalue *) parameters->head_sentinel.next;
assert(((ir_rvalue *)p)->as_rvalue() != NULL);
return (p->type->is_scalar() && p->next->is_tail_sentinel());
@@ -1008,7 +1008,7 @@ emit_inline_vector_constructor(const glsl_type *type,
*/
const unsigned lhs_components = type->components();
if (single_scalar_parameter(parameters)) {
- ir_rvalue *first_param = (ir_rvalue *)parameters->head;
+ ir_rvalue *first_param = (ir_rvalue *)parameters->head_sentinel.next;
ir_rvalue *rhs = new(ctx) ir_swizzle(first_param, 0, 0, 0, 0,
lhs_components);
ir_dereference_variable *lhs = new(ctx) ir_dereference_variable(var);
@@ -1209,7 +1209,7 @@ emit_inline_matrix_constructor(const glsl_type *type,
*to the upper left portion of the constructed matrix, and the remaining
*elements take values from the identity matrix.
*/
- ir_rvalue *const first_param = (ir_rvalue *) parameters->head;
+ ir_rvalue *const first_param = (ir_rvalue *) parameters->head_sentinel.next;
if (single_scalar_parameter(parameters)) {
/* Assign the scalar to the X component of a vec4, and fill the remaining
* components with zero.
@@ -1454,7 +1454,7 @@ emit_inline_record_constructor(const glsl_type *type,
instructions->push_tail(var);
- exec_node *node = parameters->head;
+ exec_node *node = parameters->head_sentinel.next;
for (unsigned i = 0; i < type->length; i++) {
assert(!node->is_tail_sentinel());
@@ -1487,7 +1487,7 @@ process_record_constructor(exec_list *instructions,
process_parameters(instructions, &actual_parameters,
parameters, state);
- exec_node *node = actual_parameters.head;
+ exec_node *node = actual_parameters.head_sentinel.next;
for (unsigned i = 0; i < constructor_type->length; i++) {
ir_rvalue *ir = (ir_rvalue *) node;
@@ -1751,7 +1751,7 @@ ast_function_expression::hir(exec_list *instructions,
if (all_parameters_are_constant) {
return new(ctx) ir_constant(constructor_type, &actual_parameters);
} else if (constructor_type->is_scalar()) {
- return dereference_component((ir_rvalue *) actual_parameters.head,
+ return dereference_component((ir_rvalue *) actual_parameters.head_sentinel.next,
0);
} else if (constructor_type->is_vector()) {
return emi
Re: [Mesa-dev] [PATCH] mesa/es3.1: Enable GL_ARB_separate_shader_objects for GLES 3.1
On Tue, Jun 23, 2015 at 4:30 AM, Marta Lofstedt
wrote:
> From: Marta Lofstedt
>
> Signed-off-by: Marta Lofstedt
> ---
> src/mesa/main/get_hash_params.py | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/main/get_hash_params.py
> b/src/mesa/main/get_hash_params.py
> index 74ff3ba..f7134a9 100644
> --- a/src/mesa/main/get_hash_params.py
> +++ b/src/mesa/main/get_hash_params.py
> @@ -407,6 +407,10 @@ descriptor=[
> { "apis": ["GL", "GL_CORE", "GLES3"], "params": [
> # GL_ARB_sampler_objects / GL 3.3 / GLES 3.0
>[ "SAMPLER_BINDING", "LOC_CUSTOM, TYPE_INT, GL_SAMPLER_BINDING, NO_EXTRA"
> ],
> +
> +# GL_ARB_vertex_attrib_binding / GLES 3.1
> + [ "MAX_VERTEX_ATTRIB_RELATIVE_OFFSET",
> "CONTEXT_ENUM(Const.MaxVertexAttribRelativeOffset), NO_EXTRA" ],
> + [ "MAX_VERTEX_ATTRIB_BINDINGS",
> "CONTEXT_ENUM(Const.MaxVertexAttribBindings), NO_EXTRA" ],
Won't this enable it in ES 3.0 as well? I think you need to replace
NO_EXTRA with something that contains EXTRA_API_ES31. See
extra_ARB_draw_indirect_es31 for an example.
> ]},
>
> # Enums in OpenGL Core profile and ES 3.1
> @@ -776,10 +780,6 @@ descriptor=[
>[ "MAX_COMBINED_ATOMIC_COUNTER_BUFFERS",
> "CONTEXT_INT(Const.MaxCombinedAtomicBuffers),
> extra_ARB_shader_atomic_counters" ],
>[ "MAX_COMBINED_ATOMIC_COUNTERS",
> "CONTEXT_INT(Const.MaxCombinedAtomicCounters),
> extra_ARB_shader_atomic_counters" ],
>
> -# GL_ARB_vertex_attrib_binding
> - [ "MAX_VERTEX_ATTRIB_RELATIVE_OFFSET",
> "CONTEXT_ENUM(Const.MaxVertexAttribRelativeOffset), NO_EXTRA" ],
> - [ "MAX_VERTEX_ATTRIB_BINDINGS",
> "CONTEXT_ENUM(Const.MaxVertexAttribBindings), NO_EXTRA" ],
> -
> # GL_ARB_shader_image_load_store
>[ "MAX_IMAGE_UNITS", "CONTEXT_INT(Const.MaxImageUnits),
> extra_ARB_shader_image_load_store"],
>[ "MAX_COMBINED_IMAGE_UNITS_AND_FRAGMENT_OUTPUTS",
> "CONTEXT_INT(Const.MaxCombinedImageUnitsAndFragmentOutputs),
> extra_ARB_shader_image_load_store"],
> --
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 91077] dri2_glx.c:1186: undefined reference to `loader_open_device'
https://bugs.freedesktop.org/show_bug.cgi?id=91077 --- Comment #2 from Vinson Lee --- Clean checkout and build on CentOS 6. $ ./autogen.sh --disable-dri3 --enable-sysfs --with-dri-drivers=swrast --with-gallium-drivers= $ make [...] CC XF86dri.lo dri_common.c: In function ‘dri2_convert_glx_attribs’: dri_common.c:469: warning: ‘profile’ may be used uninitialized in this function CCLD libglx.la CCLD libGL.la ./.libs/libglx.a(dri2_glx.o): In function `dri2CreateScreen': src/glx/dri2_glx.c:1186: undefined reference to `loader_open_device' collect2: ld returned 1 exit status -- 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 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] Fix strict-aliasing violations in GLSL shader list implementation
On Wed, Jun 24, 2015 at 5:05 PM, Davin McCall wrote: > Hi - I'm new here. > > I've recently started poking the Mesa codebase for little reason other than > personal interest. In the "help wanted" section of the website it mentions > aliasing violations as a target for newcomers to fix, so with that in mind > I've attached a patch (against git head) which resolves a few of them, by > targeting the linked list implementation (list.h) used in the GLSL > compiler/optimizers. This change slightly increases the storage requirements > for a list (adds one word) but resolves the blatant aliasing violation that > was caused by the trick used to conserve that word in the first place. > > (I toyed with another approach - using a single sentinel node for both the > head and tail of a list - but this was much more invasive, and meant that > you could no longer check whether a particular node was a sentinel node > unless you had a reference to the list, so I gave up and went with this > simpler approach). > > The most essential change is in the 'exec_list' structure. Three fields > 'head', 'tail' and 'tail_pred' are removed, and two separate sentinel nodes > are inserted in their place. The old 'head' is replaced by > 'head_sentinel.next', 'tail_pred' by 'tail_sentinel.prev', and tail (always > NULL) by 'head_sentinel.prev' and 'tail_sentinel.next' (both always NULL). > > With this patch, I can build a working (though perhaps not 100% bug-free) > Mesa without using -fno-strict-aliasing during compilation. Before the > patch, applications using Mesa would hang at runtime. > > I don't really know the process you have in place so if I need to do > anything else to get this patch into the codebase please let me know. Any > comments and criticisms welcome. The biggest part of the process is to send a proper commit to the mailing list. There are two issues with your mailing (without commenting on your actual changes) -- (a) The patch is attached, not inlined (b) You sent a diff, not a commit Both of these are easily resolved by using 'git send-email' for your patches, but that's not a strict requirement. See http://mesa3d.org/devinfo.html#submitting . As for feedback on your actual patch, you may want to explain why you did the things you did, what the old world was and what the new world is, and why the old world was wrong. You did that to some extent, and perhaps I'm just too unfamiliar with exec_list to make sense of it while others will "get it". Perhaps specific warnings generated by GCC may be able to better illustrate what problem you're trying to solve. Since you're increasing the storage requirements of a pretty basic unit of storage in mesa, it may also be interesting to see memory usage of some non-trivial trace or game before & after. While I think that's optional, it's definitely a nice-to-have. Cheers, -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] Fix strict-aliasing violations in GLSL shader list implementation
On Wed, Jun 24, 2015 at 2:05 PM, Davin McCall wrote: > Hi - I'm new here. Welcome! > I've recently started poking the Mesa codebase for little reason other than > personal interest. In the "help wanted" section of the website it mentions > aliasing violations as a target for newcomers to fix, so with that in mind > I've attached a patch (against git head) which resolves a few of them, by > targeting the linked list implementation (list.h) used in the GLSL > compiler/optimizers. This change slightly increases the storage requirements > for a list (adds one word) but resolves the blatant aliasing violation that > was caused by the trick used to conserve that word in the first place. > > (I toyed with another approach - using a single sentinel node for both the > head and tail of a list - but this was much more invasive, and meant that > you could no longer check whether a particular node was a sentinel node > unless you had a reference to the list, so I gave up and went with this > simpler approach). > > The most essential change is in the 'exec_list' structure. Three fields > 'head', 'tail' and 'tail_pred' are removed, and two separate sentinel nodes > are inserted in their place. The old 'head' is replaced by > 'head_sentinel.next', 'tail_pred' by 'tail_sentinel.prev', and tail (always > NULL) by 'head_sentinel.prev' and 'tail_sentinel.next' (both always NULL). > > With this patch, I can build a working (though perhaps not 100% bug-free) > Mesa without using -fno-strict-aliasing during compilation. Before the > patch, applications using Mesa would hang at runtime. Thanks for the patch. I'm impressed that fixing exec_list/exec_node allows the removal of -fno-strict-aliasing (at least, we don't know of the rest of the bugs yet :). To summarize your change, instead of the head/tail/tail_pred pointers with "tail" shared between two aliased exec_nodes, you've simply allocated two sentinel exec_nodes inside exec_list directly. Like Ilia says, some memory usage statistics before/after your patch would be very welcome. There's an apitrace of Dota2 that we've often used for memory usage testing (via valgrind massif): http://people.freedesktop.org/~anholt/dota_linux.trace > I don't really know the process you have in place so if I need to do > anything else to get this patch into the codebase please let me know. Any > comments and criticisms welcome. Please send patches inline using git send-email with a commit title and message that looks similar to others. In this case, the patch should be titled something like "glsl: Modify exec_list to avoid strict-aliasing violations." If you wanted to go farther and remove -fno-strict-aliasing from configure.ac -- Since strict-aliasing allows some additional optimization opportunities, I'd expect some justification for the patch such as the output of the 'size' command on the resulting binaries, like in this commit: commit d35720da9b9824d104532028775e497491f433ad Author: Matt Turner Date: Wed Mar 4 17:27:21 2015 -0800 i965: Mark paths in linear <-> tiled functions as unreachable(). textdata bss dec hex filename 9663 0 0966325bf intel_tiled_memcpy.o before 8215 0 082152017 intel_tiled_memcpy.o after Presumably if strict-aliasing indeed allows additional optimizations, we'd see smaller .text sizes after the removal of -fno-strict-aliasing. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] Fix strict-aliasing violations in GLSL shader list implementation
2015-06-24 23:05 GMT+02:00 Davin McCall : > Hi - I'm new here. > > I've recently started poking the Mesa codebase for little reason other than > personal interest. In the "help wanted" section of the website it mentions > aliasing violations as a target for newcomers to fix, so with that in mind > I've attached a patch (against git head) which resolves a few of them, by > targeting the linked list implementation (list.h) used in the GLSL > compiler/optimizers. This change slightly increases the storage requirements > for a list (adds one word) but resolves the blatant aliasing violation that > was caused by the trick used to conserve that word in the first place. > > (I toyed with another approach - using a single sentinel node for both the > head and tail of a list - but this was much more invasive, and meant that > you could no longer check whether a particular node was a sentinel node > unless you had a reference to the list, so I gave up and went with this > simpler approach). > > The most essential change is in the 'exec_list' structure. Three fields > 'head', 'tail' and 'tail_pred' are removed, and two separate sentinel nodes > are inserted in their place. The old 'head' is replaced by > 'head_sentinel.next', 'tail_pred' by 'tail_sentinel.prev', and tail (always > NULL) by 'head_sentinel.prev' and 'tail_sentinel.next' (both always NULL). > > With this patch, I can build a working (though perhaps not 100% bug-free) > Mesa without using -fno-strict-aliasing during compilation. Before the > patch, applications using Mesa would hang at runtime. > > I don't really know the process you have in place so if I need to do > anything else to get this patch into the codebase please let me know. Any > comments and criticisms welcome. > > Davin > > Hi Davin, Welcome aboard the Mesa-boat! I haven't really looked at your patch (I'll leave that to someone who knows the code), but here's a few pointers on how things work around here. (I see Ilia already answered you) Have a look at http://www.mesa3d.org/devinfo.html. This has most of the info you need on how to send patches and how your source code should be formatted. We try to limit lines to aprox 78 chars (no more than 80). Patches also should have a "location" for the change in the subject. (In your patch: [PATCH] glsl: Fix strict-aliasing...) And subjects in present tense, but you got that right =) Until you get commit access someone else will have to push the patches to upstream for you (usually the reviewer). That's a few points of the top of my head. If there's anything else feel free to ask. I'm not the most experienced guy in here, but I guess I'm at least able to answer most of your practical questions =) -Thomas ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 5/5] i965/gen9: Allocate YF/YS tiled buffer objects
On Tue, Jun 23, 2015 at 01:23:05PM -0700, Anuj Phogat wrote:
> In case of I915_TILING_{X,Y} we need to pass tiling format to libdrm
> using drm_intel_bo_alloc_tiled(). But, In case of YF/YS tiled buffers
> libdrm need not know about the tiling format because these buffers
> don't have hardware support to be tiled or detiled through a fenced
> region. libdrm still need to know buffer alignment value for its use
> in kernel when resolving the relocation.
>
> Using drm_intel_bo_alloc_for_render() for YF/YS tiled buffers
> satisfy both the above conditions.
>
> V2: Delete min/max buffer size restrictions not valid for i965+.
> Remove redundant align to tile size statements.
> Remove some redundant code now when there are no min/max buffer size.
>
> Signed-off-by: Anuj Phogat
> Cc: Ben Widawsky
> ---
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 62
> +--
> 1 file changed, 58 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 80c52f2..5bcb094 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -558,6 +558,48 @@ intel_lower_compressed_format(struct brw_context *brw,
> mesa_format format)
> }
> }
>
> +/* This function computes Yf/Ys tiled bo size, alignment and pitch. */
> +static uint64_t
> +intel_get_yf_ys_bo_size(struct intel_mipmap_tree *mt, unsigned *alignment,
> +uint64_t *pitch)
> +{
> + const uint32_t bpp = mt->cpp * 8;
> + const uint32_t aspect_ratio = (bpp == 16 || bpp == 64) ? 2 : 1;
> + uint32_t tile_width, tile_height;
> + uint64_t stride, size, aligned_y;
> +
> + assert(mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE);
> +
> + *alignment = mt->tr_mode == INTEL_MIPTREE_TRMODE_YF ? 4096 : 64 * 1024;
> +
> + switch (bpp) {
> + case 8:
> + tile_height = 64;
> + break;
> + case 16:
> + case 32:
> + tile_height = 32;
> + break;
> + case 64:
> + case 128:
> + tile_height = 16;
> + break;
> + default:
> + unreachable("not reached");
> + }
> +
> + if (mt->tr_mode == INTEL_MIPTREE_TRMODE_YS)
> + tile_height *= 4;
> +
> + aligned_y = ALIGN(mt->total_height, tile_height);
> + stride = mt->total_width * mt->cpp;
> + tile_width = tile_height * mt->cpp * aspect_ratio;
> + stride = ALIGN(stride, tile_width);
> + size = stride * aligned_y;
> +
> + *pitch = stride;
> + return size;
> +}
>
> struct intel_mipmap_tree *
> intel_miptree_create(struct brw_context *brw,
> @@ -616,11 +658,23 @@ intel_miptree_create(struct brw_context *brw,
>alloc_flags |= BO_ALLOC_FOR_RENDER;
>
> unsigned long pitch;
> - mt->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "miptree", total_width,
> - total_height, mt->cpp, &mt->tiling,
> - &pitch, alloc_flags);
> mt->etc_format = etc_format;
> - mt->pitch = pitch;
> +
> + if (mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE) {
> + unsigned alignment = 0;
> + unsigned long size;
> + size = intel_get_yf_ys_bo_size(mt, &alignment, &pitch);
> + assert(size);
> + mt->bo = drm_intel_bo_alloc_for_render(brw->bufmgr, "miptree",
> + size, alignment);
> + mt->pitch = pitch;
> + } else {
> + mt->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "miptree",
> +total_width, total_height, mt->cpp,
> +&mt->tiling, &pitch,
> +alloc_flags);
> + mt->pitch = pitch;
> + }
>
> /* If the BO is too large to fit in the aperture, we need to use the
> * BLT engine to support it. Prior to Sandybridge, the BLT paths can't
>
You could move mt->pitch = pitch outside of the if/else (or get rid of the local
variable entirely):
Reviewed-by: Ben Widawsky
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] loader: move loader_open_device out of HAVE_LIBUDEV block
Fixes:
CCLD libGL.la
./.libs/libglx.a(dri2_glx.o): In function `dri2CreateScreen':
src/glx/dri2_glx.c:1186: undefined reference to `loader_open_device'
collect2: ld returned 1 exit status
CCLD libEGL.la
Undefined symbols for architecture x86_64:
"_loader_open_device", referenced from:
_dri2_initialize_x11_dri2 in libegl_dri2.a(platform_x11.o)
https://bugs.freedesktop.org/show_bug.cgi?id=91077
Signed-off-by: Julien Isorce
---
src/loader/loader.c | 36 ++--
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/src/loader/loader.c b/src/loader/loader.c
index fc46815..8452cd3 100644
--- a/src/loader/loader.c
+++ b/src/loader/loader.c
@@ -64,6 +64,8 @@
*Rob Clark
*/
+#include
+#include
#include
#include
#include
@@ -71,10 +73,8 @@
#ifdef HAVE_LIBUDEV
#include
#include
-#include
#include
#include
-#include
#ifdef USE_DRICONF
#include "xmlconfig.h"
#include "xmlpool.h"
@@ -104,6 +104,22 @@ static void default_logger(int level, const char *fmt, ...)
static void (*log_)(int level, const char *fmt, ...) = default_logger;
+int
+loader_open_device(const char *device_name)
+{
+ int fd;
+#ifdef O_CLOEXEC
+ fd = open(device_name, O_RDWR | O_CLOEXEC);
+ if (fd == -1 && errno == EINVAL)
+#endif
+ {
+ fd = open(device_name, O_RDWR);
+ if (fd != -1)
+ fcntl(fd, F_SETFD, fcntl(fd, F_GETFD) | FD_CLOEXEC);
+ }
+ return fd;
+}
+
#ifdef HAVE_LIBUDEV
#include
@@ -314,22 +330,6 @@ get_id_path_tag_from_fd(struct udev *udev, int fd)
return id_path_tag;
}
-int
-loader_open_device(const char *device_name)
-{
- int fd;
-#ifdef O_CLOEXEC
- fd = open(device_name, O_RDWR | O_CLOEXEC);
- if (fd == -1 && errno == EINVAL)
-#endif
- {
- fd = open(device_name, O_RDWR);
- if (fd != -1)
- fcntl(fd, F_SETFD, fcntl(fd, F_GETFD) | FD_CLOEXEC);
- }
- return fd;
-}
-
#ifdef USE_DRICONF
const char __driConfigOptionsLoader[] =
DRI_CONF_BEGIN
--
1.9.5 (Apple Git-50.3)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 91077] dri2_glx.c:1186: undefined reference to `loader_open_device'
https://bugs.freedesktop.org/show_bug.cgi?id=91077 --- Comment #3 from Julien Isorce --- Hi, I just sent a patch on the mailing list as I encountered the same problem on darwin. -- 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 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] configure: use $target_cpu, not $host_cpu when setting asm_arch
Otherwise, if we're trying to build a 32-bit Mesa on a 64-bit host we wind up with -DUSE_X86_64_ASM, which is incorrect. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index ddc757e..b12f5f9 100644 --- a/configure.ac +++ b/configure.ac @@ -605,7 +605,7 @@ if test "x$enable_asm" = xyes -a "x$cross_compiling" = xyes; then fi # check for supported arches if test "x$enable_asm" = xyes; then -case "$host_cpu" in +case "$target_cpu" in i?86) case "$host_os" in linux* | *freebsd* | dragonfly* | *netbsd* | openbsd* | gnu*) -- 1.9.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] configure: use $target_cpu, not $host_cpu when setting asm_arch
On Tue, Jun 23, 2015 at 3:04 PM, Brian Paul wrote: > Otherwise, if we're trying to build a 32-bit Mesa on a 64-bit host > we wind up with -DUSE_X86_64_ASM, which is incorrect. > --- > configure.ac | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/configure.ac b/configure.ac > index ddc757e..b12f5f9 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -605,7 +605,7 @@ if test "x$enable_asm" = xyes -a "x$cross_compiling" = > xyes; then > fi > # check for supported arches > if test "x$enable_asm" = xyes; then > -case "$host_cpu" in > +case "$target_cpu" in > i?86) > case "$host_os" in > linux* | *freebsd* | dragonfly* | *netbsd* | openbsd* | gnu*) > -- > 1.9.1 According to [1], host is "the machine that you are building for" and target is "the machine that GCC will produce code for". In the context of building GCC, I think this means that the resulting GCC binaries will run on $host and will produce code for $target. In the context of Mesa, I can't come up with a way that host != target makes sense. docs/autoconf.html suggests using --build=x86_64-pc-linux-gnu --host=i686-pc-linux-gnu to build on x86_64 for i686. Is that what you're doing? [1] https://gcc.gnu.org/onlinedocs/gccint/Configure-Terms.html ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] Fix strict-aliasing violations in GLSL shader list implementation
On 06/23/2015 02:36 PM, Thomas Helland wrote: > 2015-06-24 23:05 GMT+02:00 Davin McCall : >> Hi - I'm new here. >> >> I've recently started poking the Mesa codebase for little reason other than >> personal interest. In the "help wanted" section of the website it mentions >> aliasing violations as a target for newcomers to fix, so with that in mind >> I've attached a patch (against git head) which resolves a few of them, by >> targeting the linked list implementation (list.h) used in the GLSL >> compiler/optimizers. This change slightly increases the storage requirements >> for a list (adds one word) but resolves the blatant aliasing violation that >> was caused by the trick used to conserve that word in the first place. >> >> (I toyed with another approach - using a single sentinel node for both the >> head and tail of a list - but this was much more invasive, and meant that >> you could no longer check whether a particular node was a sentinel node >> unless you had a reference to the list, so I gave up and went with this >> simpler approach). >> >> The most essential change is in the 'exec_list' structure. Three fields >> 'head', 'tail' and 'tail_pred' are removed, and two separate sentinel nodes >> are inserted in their place. The old 'head' is replaced by >> 'head_sentinel.next', 'tail_pred' by 'tail_sentinel.prev', and tail (always >> NULL) by 'head_sentinel.prev' and 'tail_sentinel.next' (both always NULL). NAK. The datastructure is correct as-is. It has been in common use since at least 1985. See the references in the header file. >> With this patch, I can build a working (though perhaps not 100% bug-free) >> Mesa without using -fno-strict-aliasing during compilation. Before the >> patch, applications using Mesa would hang at runtime. >> >> I don't really know the process you have in place so if I need to do >> anything else to get this patch into the codebase please let me know. Any >> comments and criticisms welcome. >> >> Davin >> >> > > Hi Davin, > > Welcome aboard the Mesa-boat! > I haven't really looked at your patch > (I'll leave that to someone who knows the code), > but here's a few pointers on how things work around here. > (I see Ilia already answered you) > > Have a look at http://www.mesa3d.org/devinfo.html. > This has most of the info you need on how to send patches > and how your source code should be formatted. > > We try to limit lines to aprox 78 chars (no more than 80). > Patches also should have a "location" for the change in the subject. > (In your patch: [PATCH] glsl: Fix strict-aliasing...) > And subjects in present tense, but you got that right =) > > Until you get commit access someone else will have to > push the patches to upstream for you (usually the reviewer). > > That's a few points of the top of my head. > If there's anything else feel free to ask. > I'm not the most experienced guy in here, but I guess I'm at > least able to answer most of your practical questions =) > > -Thomas > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] Fix strict-aliasing violations in GLSL shader list implementation
Hi Ilia (and Matt and Thomas) On 23/06/15 22:30, Ilia Mirkin wrote: The biggest part of the process is to send a proper commit to the mailing list. There are two issues with your mailing (without commenting on your actual changes) -- (a) The patch is attached, not inlined (b) You sent a diff, not a commit Both of these are easily resolved by using 'git send-email' for your patches, but that's not a strict requirement. See http://mesa3d.org/devinfo.html#submitting . Thanks for responding! I'll look into sending the patch using 'git send-email'.I'm still fairly new to git. Since you're increasing the storage requirements of a pretty basic unit of storage in mesa, it may also be interesting to see memory usage of some non-trivial trace or game before & after. While I think that's optional, it's definitely a nice-to-have. Right, I'll see if I can produce something meaningful. Davin ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965/fs: Get rid of an unused variable in emit_barrier()
--- src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp index ea29341..9a4bad6 100644 --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp @@ -1963,11 +1963,11 @@ fs_visitor::emit_barrier() fs_reg payload = fs_reg(GRF, alloc.allocate(1), BRW_REGISTER_TYPE_UD); /* Clear the message payload */ - fs_inst *inst = bld.exec_all().MOV(payload, fs_reg(0u)); + bld.exec_all().MOV(payload, fs_reg(0u)); /* Copy bits 27:24 of r0.2 (barrier id) to the message payload reg.2 */ fs_reg r0_2 = fs_reg(retype(brw_vec1_grf(0, 2), BRW_REGISTER_TYPE_UD)); - inst = bld.exec_all().AND(component(payload, 2), r0_2, fs_reg(0x0f00u)); + bld.exec_all().AND(component(payload, 2), r0_2, fs_reg(0x0f00u)); /* Emit a gateway "barrier" message using the payload we set up, followed * by a wait instruction. -- 2.4.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 26/82] glsl: Don't do copy propagation on buffer variables
On 2015-06-22 23:38:14, Iago Toral wrote:
> On Mon, 2015-06-22 at 14:28 -0700, Jordan Justen wrote:
> > 24-26 once again makes me wonder if these optimization *can* be used
> > with SSBOs based on the same ext spec wording I referenced before:
> >
> > "The ability to write to buffer objects creates the potential for
> > multiple independent shader invocations to read and write the same
> > underlying memory. The same issue exists with the
> > ARB_shader_image_load_store extension provided in OpenGL 4.2, which
> > can write to texture objects and buffers. In both cases, the
> > specification makes few guarantees related to the relative order of
> > memory reads and writes performed by the shader invocations."
> >
> > In these patches "other threads" were specifically mentioned.
> >
> > Did these patches also prevent bad things from happening in generated
> > code? (Like mentioned for patch 23.)
>
> I think the problem here is the possibility for shaders to use
> memoryBarrier():
>
> "SHADER_STORAGE_BARRIER_BIT: Memory accesses using shader buffer
> variables issued after the barrier will reflect data written by
> shaders prior to the barrier. Additionally, assignments to and atomic
> operations performed on shader buffer variables after the barrier will
> not execute until all memory accesses (e.g., loads, stores, texture
> fetches, vertex fetches) initiated prior to the barrier complete."
>
> I think in these cases we can't allow these optimizations to kick in.
>
> That said, maybe we can check if we are using any memorybarriers in the
> shader code and decide if we want to enable these optimizations for
> ssbos based on that. I think we can try to do that in a later patch.
Ok. What do you think about updating the commit messages on these
three patches?
For example, currently you have:
"Since the backing storage for these is shared we cannot ensure that
the value won't change by writes from other threads."
How does something like this sound?
"Since the backing storage for these is shared we cannot ensure that
the value won't change by writes from other threads. Normally SSBO
accesses are not guaranteed to be syncronized with other threads,
except when memoryBarrier is used. So, we might be able to optimize
some SSBO accesses, but for now we always take the safe path and emit
the SSBO access."
With a change like that to these commit messages, you can add
Reviewed-by: Jordan Justen
to all 3 patches.
> > On 2015-06-03 00:01:16, Iago Toral Quiroga wrote:
> > > Since the backing storage for these is shared we cannot ensure that the
> > > value won't change by writes from other threads.
> > > ---
> > > src/glsl/opt_copy_propagation.cpp | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/src/glsl/opt_copy_propagation.cpp
> > > b/src/glsl/opt_copy_propagation.cpp
> > > index 806027b..f206995 100644
> > > --- a/src/glsl/opt_copy_propagation.cpp
> > > +++ b/src/glsl/opt_copy_propagation.cpp
> > > @@ -330,7 +330,7 @@ ir_copy_propagation_visitor::add_copy(ir_assignment
> > > *ir)
> > > */
> > > ir->condition = new(ralloc_parent(ir)) ir_constant(false);
> > > this->progress = true;
> > > - } else {
> > > + } else if (lhs_var->data.mode != ir_var_shader_storage) {
> > > entry = new(this->acp) acp_entry(lhs_var, rhs_var);
> > > this->acp->push_tail(entry);
> > >}
> > > --
> > > 1.9.1
> > >
> > > ___
> > > mesa-dev mailing list
> > > mesa-dev@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/mesa: remove unneeded pipe_surface_release() in st_render_texture()
Reviewed-by: Marek Olšák Marek On Tue, Jun 23, 2015 at 6:30 PM, Brian Paul wrote: > This caused us to always free the pipe_surface for the renderbuffer. > The subsequent call to st_update_renderbuffer_surface() would typically > just recreate it. Remove the call to pipe_surface_release() and let > st_update_renderbuffer_surface() take care of freeing the old surface > if it needs to be replaced (because of change to mipmap level, etc). > > This can save quite a few calls to pipe_context::create_surface() and > surface_destroy(). > --- > src/mesa/state_tracker/st_cb_fbo.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/src/mesa/state_tracker/st_cb_fbo.c > b/src/mesa/state_tracker/st_cb_fbo.c > index 0399eef..5707590 100644 > --- a/src/mesa/state_tracker/st_cb_fbo.c > +++ b/src/mesa/state_tracker/st_cb_fbo.c > @@ -511,8 +511,6 @@ st_render_texture(struct gl_context *ctx, > strb->rtt_layered = att->Layered; > pipe_resource_reference(&strb->texture, pt); > > - pipe_surface_release(pipe, &strb->surface); > - > st_update_renderbuffer_surface(st, strb); > > strb->Base.Format = st_pipe_format_to_mesa_format(pt->format); > -- > 1.9.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/fs: Get rid of an unused variable in emit_barrier()
Reviewed-by: Jordan Justen On 2015-06-23 15:40:00, Jason Ekstrand wrote: > --- > src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp > b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp > index ea29341..9a4bad6 100644 > --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp > +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp > @@ -1963,11 +1963,11 @@ fs_visitor::emit_barrier() > fs_reg payload = fs_reg(GRF, alloc.allocate(1), BRW_REGISTER_TYPE_UD); > > /* Clear the message payload */ > - fs_inst *inst = bld.exec_all().MOV(payload, fs_reg(0u)); > + bld.exec_all().MOV(payload, fs_reg(0u)); > > /* Copy bits 27:24 of r0.2 (barrier id) to the message payload reg.2 */ > fs_reg r0_2 = fs_reg(retype(brw_vec1_grf(0, 2), BRW_REGISTER_TYPE_UD)); > - inst = bld.exec_all().AND(component(payload, 2), r0_2, > fs_reg(0x0f00u)); > + bld.exec_all().AND(component(payload, 2), r0_2, fs_reg(0x0f00u)); > > /* Emit a gateway "barrier" message using the payload we set up, followed > * by a wait instruction. > -- > 2.4.3 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] configure: use $target_cpu, not $host_cpu when setting asm_arch
On 06/23/2015 04:25 PM, Matt Turner wrote: On Tue, Jun 23, 2015 at 3:04 PM, Brian Paul wrote: Otherwise, if we're trying to build a 32-bit Mesa on a 64-bit host we wind up with -DUSE_X86_64_ASM, which is incorrect. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index ddc757e..b12f5f9 100644 --- a/configure.ac +++ b/configure.ac @@ -605,7 +605,7 @@ if test "x$enable_asm" = xyes -a "x$cross_compiling" = xyes; then fi # check for supported arches if test "x$enable_asm" = xyes; then -case "$host_cpu" in +case "$target_cpu" in i?86) case "$host_os" in linux* | *freebsd* | dragonfly* | *netbsd* | openbsd* | gnu*) -- 1.9.1 According to [1], host is "the machine that you are building for" and target is "the machine that GCC will produce code for". In the context of building GCC, I think this means that the resulting GCC binaries will run on $host and will produce code for $target. In the context of Mesa, I can't come up with a way that host != target makes sense. docs/autoconf.html suggests using --build=x86_64-pc-linux-gnu --host=i686-pc-linux-gnu to build on x86_64 for i686. Is that what you're doing? Thanks for the pointer to the docs! I forgot this was mentioned there. I basically had my --host and --build mixed up. Also, I was using "i686-linux-gnu" instead of "i686-pc-linux-gnu" (is there really a difference)? Let me see how far I get now that you set me straight... -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] Fix strict-aliasing violations in GLSL shader list implementation
Hi Ian,
On 23/06/15 23:26, Ian Romanick wrote:
On 06/23/2015 02:36 PM, Thomas Helland wrote:
2015-06-24 23:05 GMT+02:00 Davin McCall :
Hi - I'm new here.
I've recently started poking the Mesa codebase for little reason other than
personal interest. In the "help wanted" section of the website it mentions
aliasing violations as a target for newcomers to fix, so with that in mind
I've attached a patch (against git head) which resolves a few of them, by
targeting the linked list implementation (list.h) used in the GLSL
compiler/optimizers. This change slightly increases the storage requirements
for a list (adds one word) but resolves the blatant aliasing violation that
was caused by the trick used to conserve that word in the first place.
(I toyed with another approach - using a single sentinel node for both the
head and tail of a list - but this was much more invasive, and meant that
you could no longer check whether a particular node was a sentinel node
unless you had a reference to the list, so I gave up and went with this
simpler approach).
The most essential change is in the 'exec_list' structure. Three fields
'head', 'tail' and 'tail_pred' are removed, and two separate sentinel nodes
are inserted in their place. The old 'head' is replaced by
'head_sentinel.next', 'tail_pred' by 'tail_sentinel.prev', and tail (always
NULL) by 'head_sentinel.prev' and 'tail_sentinel.next' (both always NULL).
NAK. The datastructure is correct as-is. It has been in common use
since at least 1985. See the references in the header file.
I understand the data structure and how it is supposed to work; the
issue is that the trick it employs cannot be implemented in C without
breaking the strict aliasing rules (or at least, the current
implementation in Mesa breaks the strict aliasing rules). The current
code works correctly only with the -fno-strict-aliasing compiler option.
The issue is that a pair of 'exec_node *' do not constitute an exec_node
in the eyes of the language spec, even though exec_node is declared as
holding two such pointers. Consider (from src/glsl/list.h):
static inline void
exec_list_make_empty(struct exec_list *list)
{
list->head = (struct exec_node *) & list->tail;
list->tail = NULL;
list->tail_pred = (struct exec_node *) & list->head;
}
'list->head' is of type 'struct exec_node *', and so should point at a
'struct exec_node'. In the code above it is instead co-erced to point
at a 'struct exec_node *' (list->tail). That in itself doesn't break the
alias rules, but then:
static inline bool
exec_node_is_tail_sentinel(const struct exec_node *n)
{
return n->next == NULL;
}
In 'exec_node_is_tail_sentinel', the sentinel is not actually an
exec_node - it is &list->tail. So, if the parameter n does refer to the
sentinel, then it does not point to an actual exec_node structure.
However, it is de-referenced (by 'n->next') which breaks the strict
aliasing rules. This means that the method above can only ever return
false, unless it violates the aliasing rules.
(The above method could be fixed by casting n to an 'struct exec_node
**' and then comparing '**n' against NULL. However, there are other
similar examples throughout the code that I do not think would be so
trivial).
I can quote the relevant parts of the standard if necessary, but your
tone somewhat implies that you wouldn't even consider this patch?
Davin
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC shader-db] Add support for shadertoy tests
I have a couple of python pointers for you, feel free to take them or
leave them.
Dylan
On Tue, Jun 16, 2015 at 03:46:50PM -0400, Rob Clark wrote:
> Attached script grabs shaders from shadertoy, and dumps them out as
> .shader_test files which can be run through shader-db for compiler
> testing.
>
> shadertoy only gives you a fragment shader (which works based on
> gl_FragCoord), so a generic vertex shader is used. And a blurb is
> inserted for the pre-defined uniforms and main() function (which just
> calls shadertoy mainImage() fxn).
>
> ---
> TODO I guess we'd actually have to parse the shader to figure out if
> the sampler uniforms were meant to be 2D/cube/etc. Maybe we just
> commit samplers we get from the script and massage them by hand?
>
> PS. don't make fun of my py too much.. I'm a newb and figuring it
> out as I go
I'm trying not to make fun, but I do have quite a few pointers for you.
>
> grab-shadertoy.py | 63
> +++
> 1 file changed, 63 insertions(+)
> create mode 100755 grab-shadertoy.py
>
> diff --git a/grab-shadertoy.py b/grab-shadertoy.py
> new file mode 100755
> index 000..74e9d10
> --- /dev/null
> +++ b/grab-shadertoy.py
> @@ -0,0 +1,63 @@
> +#!/usr/bin/env python3
> +
> +
> +import requests, json
You're not actually using json
> +
> +url = 'https://www.shadertoy.com/api/v1/shaders'
> +key = '?key=NdnKw7'
> +
> +# Get the list of shaders
> +r = requests.get(url + key)
> +j = r.json()
> +print('Found ' + str(j['Shaders']) + ' shaders')
If you use format you can avoid calling str() on everything, and make
things more readable using format rather than concatenation:
print('Found {} shaders'.format(j['Shaders']))
> +
> +shader_ids = j['Results']
> +for id in shader_ids:
> +print('Fetching shader: ' + str(id))
> +r = requests.get(url + '/' + id + key)
> +j = r.json()
> +s = j['Shader']
> +info = s['info']
> +print('Name: ' + info['name'])
> +print('Description: ' + info['description'])
> +i = 0;
python has a cool builtin called enumerate for doing this:
for i, p in enmerate(s['renderpass']):
Also, I know it's easy to forget, but python doesn't use ';' at the end
of lines, it allows them, but they look weird to pythonistas
> +for p in s['renderpass']:
> +fobj = open('shaders/shadertoy/' + str(id) + '_' + str(i) +
> '.shader_test', 'w')
with str.format this would look like:
with open('shaders/shadertoy/{}_{}.shader_test'.format(id, i), 'w') as fobj:
> +#print('Inputs: ' + str(p['inputs']))
> +#print('Outputs: ' + str(p['outputs']))
> +fobj.write('[require]\n')
> +fobj.write('GLSL >= 1.30\n')
> +fobj.write('\n');
> +fobj.write('[fragment shader]\n')
> +fobj.write('#version 130\n')
> +# Shadertoy inserts some uniforms, so we need to do the same:
> +fobj.write('uniform vec3 iResolution;\n');
> +fobj.write('uniform float iGlobalTime;\n');
> +fobj.write('uniform float iChannelTime[4];\n');
> +fobj.write('uniform vec4 iMouse;\n');
> +fobj.write('uniform vec4 iDate;\n');
> +fobj.write('uniform float iSampleRate;\n');
> +fobj.write('uniform vec3 iChannelResolution[4];\n');
> +# TODO probably need to parse the shader to figure out if
> 2d/cubemap/etc
> +fobj.write('uniform sampler2D iChannel0;\n');
> +fobj.write('uniform sampler2D iChannel1;\n');
> +fobj.write('uniform sampler2D iChannel2;\n');
> +fobj.write('uniform sampler2D iChannel3;\n');
> +# Actual shadertoy shader body:
> +fobj.write(p['code'])
> +# Shadertoy shader uses mainImage(out vec4 fragColor, in vec2
> fragCoord)
> +# so we need to insert a main:
> +fobj.write('\nvoid main() { mainImage(gl_FragColor,
> gl_FragCoord.xy); }\n')
> +fobj.write('\n\n')
> +# And a generic vertex shader:
> +fobj.write('[vertex shader]\n')
> +fobj.write('#version 130\n')
> +fobj.write('in vec2 position;\n')
> +fobj.write('\n')
> +fobj.write('void main()\n')
> +fobj.write('{\n')
> +fobj.write(' gl_Position = vec4(position, 0.0, 1.0);\n')
> +fobj.write('}\n')
> +
> +fobj.close()
> +i = 1 + i
You can simplify this quite a bit:
import textwrap
header = textwrap.dedent("""\
[require]
GLSL >= 1.30
[fragment shader]
#version 130
uniform vec3 iResolution;
uniform float iGlobalTime;
uniform float iChannelTime[4];
uniform vec4 iMouse;
uniform vec4 iDate;
uniform float iSampleRate;
uniform vec3 iChannelResolution[4];
uniform sampler2D iChannel0;
uniform sampler2D iChannel1;
uniform sampler2D iChannel2;
uniform sampler2D iChannel3;
""")
footer = textwarp.dedent("""\
void main() { mainImage(gl_FragColor, gl_FragCoord.xy); }
Re: [Mesa-dev] [RFC shader-db] Add support for shadertoy tests
On Tue, Jun 23, 2015 at 7:27 PM, Dylan Baker wrote:
> I have a couple of python pointers for you, feel free to take them or
> leave them.
cool, thanks..
What do others think about including shadertoy in shader-db? If it is
a useful thing, I'll clean up my script and re-submit..
And if we include it, should we commit the scripts we pull down for
repeatable results and just keep the script as something to resync and
pull in new shaders? (Esp. given that a small percentage need some
hand massaging.. I haven't figured out a good way to
reliably/programatically figure out if the samplers should actually be
2d/3d/cube..)
That said, I think the shaderdb shaders are a fairly, umm, unique
stress test of a shader compiler.. and the API to scrape shaders seems
to convenient to ignore..
BR,
-R
> Dylan
>
> On Tue, Jun 16, 2015 at 03:46:50PM -0400, Rob Clark wrote:
>> Attached script grabs shaders from shadertoy, and dumps them out as
>> .shader_test files which can be run through shader-db for compiler
>> testing.
>>
>> shadertoy only gives you a fragment shader (which works based on
>> gl_FragCoord), so a generic vertex shader is used. And a blurb is
>> inserted for the pre-defined uniforms and main() function (which just
>> calls shadertoy mainImage() fxn).
>>
>> ---
>> TODO I guess we'd actually have to parse the shader to figure out if
>> the sampler uniforms were meant to be 2D/cube/etc. Maybe we just
>> commit samplers we get from the script and massage them by hand?
>>
>> PS. don't make fun of my py too much.. I'm a newb and figuring it
>> out as I go
>
> I'm trying not to make fun, but I do have quite a few pointers for you.
>
>>
>> grab-shadertoy.py | 63
>> +++
>> 1 file changed, 63 insertions(+)
>> create mode 100755 grab-shadertoy.py
>>
>> diff --git a/grab-shadertoy.py b/grab-shadertoy.py
>> new file mode 100755
>> index 000..74e9d10
>> --- /dev/null
>> +++ b/grab-shadertoy.py
>> @@ -0,0 +1,63 @@
>> +#!/usr/bin/env python3
>> +
>> +
>> +import requests, json
>
> You're not actually using json
>
>> +
>> +url = 'https://www.shadertoy.com/api/v1/shaders'
>> +key = '?key=NdnKw7'
>> +
>> +# Get the list of shaders
>> +r = requests.get(url + key)
>> +j = r.json()
>> +print('Found ' + str(j['Shaders']) + ' shaders')
>
> If you use format you can avoid calling str() on everything, and make
> things more readable using format rather than concatenation:
> print('Found {} shaders'.format(j['Shaders']))
>
>> +
>> +shader_ids = j['Results']
>> +for id in shader_ids:
>> +print('Fetching shader: ' + str(id))
>> +r = requests.get(url + '/' + id + key)
>> +j = r.json()
>> +s = j['Shader']
>> +info = s['info']
>> +print('Name: ' + info['name'])
>> +print('Description: ' + info['description'])
>> +i = 0;
>
> python has a cool builtin called enumerate for doing this:
> for i, p in enmerate(s['renderpass']):
>
> Also, I know it's easy to forget, but python doesn't use ';' at the end
> of lines, it allows them, but they look weird to pythonistas
>
>> +for p in s['renderpass']:
>> +fobj = open('shaders/shadertoy/' + str(id) + '_' + str(i) +
>> '.shader_test', 'w')
>
> with str.format this would look like:
> with open('shaders/shadertoy/{}_{}.shader_test'.format(id, i), 'w') as fobj:
>
>> +#print('Inputs: ' + str(p['inputs']))
>> +#print('Outputs: ' + str(p['outputs']))
>> +fobj.write('[require]\n')
>> +fobj.write('GLSL >= 1.30\n')
>> +fobj.write('\n');
>> +fobj.write('[fragment shader]\n')
>> +fobj.write('#version 130\n')
>> +# Shadertoy inserts some uniforms, so we need to do the same:
>> +fobj.write('uniform vec3 iResolution;\n');
>> +fobj.write('uniform float iGlobalTime;\n');
>> +fobj.write('uniform float iChannelTime[4];\n');
>> +fobj.write('uniform vec4 iMouse;\n');
>> +fobj.write('uniform vec4 iDate;\n');
>> +fobj.write('uniform float iSampleRate;\n');
>> +fobj.write('uniform vec3 iChannelResolution[4];\n');
>> +# TODO probably need to parse the shader to figure out if
>> 2d/cubemap/etc
>> +fobj.write('uniform sampler2D iChannel0;\n');
>> +fobj.write('uniform sampler2D iChannel1;\n');
>> +fobj.write('uniform sampler2D iChannel2;\n');
>> +fobj.write('uniform sampler2D iChannel3;\n');
>> +# Actual shadertoy shader body:
>> +fobj.write(p['code'])
>> +# Shadertoy shader uses mainImage(out vec4 fragColor, in vec2
>> fragCoord)
>> +# so we need to insert a main:
>> +fobj.write('\nvoid main() { mainImage(gl_FragColor,
>> gl_FragCoord.xy); }\n')
>> +fobj.write('\n\n')
>> +# And a generic vertex shader:
>> +fobj.write('[vertex shader]\n')
>> +fobj.write('#version 130\n')
>> +fobj.write('in vec2 position;\n')
>> +fobj.write('\n')
>> +
Re: [Mesa-dev] [PATCH] Fix strict-aliasing violations in GLSL shader list implementation
On 24.06.2015 06:35, Matt Turner wrote: > On Wed, Jun 24, 2015 at 2:05 PM, Davin McCall wrote: > >> I've recently started poking the Mesa codebase for little reason other than >> personal interest. In the "help wanted" section of the website it mentions >> aliasing violations as a target for newcomers to fix, so with that in mind >> I've attached a patch (against git head) which resolves a few of them, by >> targeting the linked list implementation (list.h) used in the GLSL >> compiler/optimizers. This change slightly increases the storage requirements >> for a list (adds one word) but resolves the blatant aliasing violation that >> was caused by the trick used to conserve that word in the first place. >> >> (I toyed with another approach - using a single sentinel node for both the >> head and tail of a list - but this was much more invasive, and meant that >> you could no longer check whether a particular node was a sentinel node >> unless you had a reference to the list, so I gave up and went with this >> simpler approach). >> >> The most essential change is in the 'exec_list' structure. Three fields >> 'head', 'tail' and 'tail_pred' are removed, and two separate sentinel nodes >> are inserted in their place. The old 'head' is replaced by >> 'head_sentinel.next', 'tail_pred' by 'tail_sentinel.prev', and tail (always >> NULL) by 'head_sentinel.prev' and 'tail_sentinel.next' (both always NULL). >> >> With this patch, I can build a working (though perhaps not 100% bug-free) >> Mesa without using -fno-strict-aliasing during compilation. Before the >> patch, applications using Mesa would hang at runtime. > > Thanks for the patch. I'm impressed that fixing exec_list/exec_node > allows the removal of -fno-strict-aliasing (at least, we don't know of > the rest of the bugs yet :). Actually, I'm almost 100% certain that there are lots of other strict aliasing violations in the Mesa code. That's why we've always disabled it. More generally, IMO it's unrealistic to rely on strict aliasing for optimization, because very few people really understand it (I'm not one of them). -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH mesa] i965/gen8+: bo in state base address must be in 32-bit address range
Gen8+ supports 48-bit virtual addresses, but some objects must always be
allocated inside the 32-bit address range.
In specific, any resource used with flat/heapless (0x-0xf000)
General State Heap (GSH) or Intruction State Heap (ISH) must be in a
32-bit range, because the General State Offset and Instruction State Offset
are limited to 32-bits.
Set provided bo flag when the 4GB limit is not necessary, to be able to use
the full address space.
Cc: mesa-dev@lists.freedesktop.org
Signed-off-by: Michel Thierry
---
src/mesa/drivers/dri/i965/gen8_misc_state.c | 6 +++---
src/mesa/drivers/dri/i965/intel_batchbuffer.h | 7 +++
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/gen8_misc_state.c
b/src/mesa/drivers/dri/i965/gen8_misc_state.c
index b20038e..26531d0 100644
--- a/src/mesa/drivers/dri/i965/gen8_misc_state.c
+++ b/src/mesa/drivers/dri/i965/gen8_misc_state.c
@@ -41,17 +41,17 @@ void gen8_upload_state_base_address(struct brw_context *brw)
OUT_BATCH(0);
OUT_BATCH(mocs_wb << 16);
/* Surface state base address: */
- OUT_RELOC64(brw->batch.bo, I915_GEM_DOMAIN_SAMPLER, 0,
+ OUT_RELOC64_32BWA(brw->batch.bo, I915_GEM_DOMAIN_SAMPLER, 0,
mocs_wb << 4 | 1);
/* Dynamic state base address: */
- OUT_RELOC64(brw->batch.bo,
+ OUT_RELOC64_32BWA(brw->batch.bo,
I915_GEM_DOMAIN_RENDER | I915_GEM_DOMAIN_INSTRUCTION, 0,
mocs_wb << 4 | 1);
/* Indirect object base address: MEDIA_OBJECT data */
OUT_BATCH(mocs_wb << 4 | 1);
OUT_BATCH(0);
/* Instruction base address: shader kernels (incl. SIP) */
- OUT_RELOC64(brw->cache.bo, I915_GEM_DOMAIN_INSTRUCTION, 0,
+ OUT_RELOC64_32BWA(brw->cache.bo, I915_GEM_DOMAIN_INSTRUCTION, 0,
mocs_wb << 4 | 1);
/* General state buffer size */
diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.h
b/src/mesa/drivers/dri/i965/intel_batchbuffer.h
index 7bdd836..5aa741e 100644
--- a/src/mesa/drivers/dri/i965/intel_batchbuffer.h
+++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.h
@@ -177,6 +177,13 @@ intel_batchbuffer_advance(struct brw_context *brw)
/* Handle 48-bit address relocations for Gen8+ */
#define OUT_RELOC64(buf, read_domains, write_domain, delta) do { \
+ drm_intel_bo_set_supports_48baddress(buf); \
+ intel_batchbuffer_emit_reloc64(brw, buf, read_domains, write_domain,
delta);\
+} while (0)
+
+/* Handle 48-bit address relocations for Gen8+, ask for 32-bit address */
+#define OUT_RELOC64_32BWA(buf, read_domains, write_domain, delta) do { \
+ drm_intel_bo_clear_supports_48baddress(buf); \
intel_batchbuffer_emit_reloc64(brw, buf, read_domains, write_domain,
delta);\
} while (0)
--
2.4.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] Fix strict-aliasing violations in GLSL shader list implementation
On 06/24/2015 03:59 PM, Davin McCall wrote:
> Hi Ian,
>
> On 23/06/15 23:26, Ian Romanick wrote:
>> On 06/23/2015 02:36 PM, Thomas Helland wrote:
>>> 2015-06-24 23:05 GMT+02:00 Davin McCall :
Hi - I'm new here.
I've recently started poking the Mesa codebase for little reason other than
personal interest. In the "help wanted" section of the website it mentions
aliasing violations as a target for newcomers to fix, so with that in mind
I've attached a patch (against git head) which resolves a few of them, by
targeting the linked list implementation (list.h) used in the GLSL
compiler/optimizers. This change slightly increases the storage
requirements
for a list (adds one word) but resolves the blatant aliasing violation that
was caused by the trick used to conserve that word in the first place.
(I toyed with another approach - using a single sentinel node for both the
head and tail of a list - but this was much more invasive, and meant that
you could no longer check whether a particular node was a sentinel node
unless you had a reference to the list, so I gave up and went with this
simpler approach).
The most essential change is in the 'exec_list' structure. Three fields
'head', 'tail' and 'tail_pred' are removed, and two separate sentinel nodes
are inserted in their place. The old 'head' is replaced by
'head_sentinel.next', 'tail_pred' by 'tail_sentinel.prev', and tail (always
NULL) by 'head_sentinel.prev' and 'tail_sentinel.next' (both always NULL).
>> NAK. The datastructure is correct as-is. It has been in common use
>> since at least 1985. See the references in the header file.
>
> I understand the data structure and how it is supposed to work; the
> issue is that the trick it employs cannot be implemented in C without
> breaking the strict aliasing rules (or at least, the current
> implementation in Mesa breaks the strict aliasing rules). The current
> code works correctly only with the -fno-strict-aliasing compiler option.
> The issue is that a pair of 'exec_node *' do not constitute an exec_node
> in the eyes of the language spec, even though exec_node is declared as
> holding two such pointers. Consider (from src/glsl/list.h):
>
> static inline void
> exec_list_make_empty(struct exec_list *list)
> {
>list->head = (struct exec_node *) & list->tail;
>list->tail = NULL;
>list->tail_pred = (struct exec_node *) & list->head;
> }
>
>
> 'list->head' is of type 'struct exec_node *', and so should point at a
> 'struct exec_node'. In the code above it is instead co-erced to point
> at a 'struct exec_node *' (list->tail). That in itself doesn't break the
> alias rules, but then:
>
> static inline bool
> exec_node_is_tail_sentinel(const struct exec_node *n)
> {
>return n->next == NULL;
> }
>
>
> In 'exec_node_is_tail_sentinel', the sentinel is not actually an
> exec_node - it is &list->tail. So, if the parameter n does refer to the
> sentinel, then it does not point to an actual exec_node structure.
> However, it is de-referenced (by 'n->next') which breaks the strict
> aliasing rules. This means that the method above can only ever return
> false, unless it violates the aliasing rules.
>
> (The above method could be fixed by casting n to an 'struct exec_node
> **' and then comparing '**n' against NULL. However, there are other
> similar examples throughout the code that I do not think would be so
> trivial).
>
> I can quote the relevant parts of the standard if necessary, but your
> tone somewhat implies that you wouldn't even consider this patch?
Please quote the spec. I have a hard time believing that the compiler
magically decides that an exec_node* is not really an exec_node* and
just does whatever it wants (which sounds like what you're describing).
That sounds pretty rubbish... and I'm surprised that anything works in
such an environment.
Mesa has also had -fno-strict-aliasing for as long as I can remember,
and this structure has only been used here for a few years. The whole
thing just doesn't smell right.
> Davin
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC shader-db] Add support for shadertoy tests
On Tue, Jun 23, 2015 at 08:18:45PM -0400, Rob Clark wrote:
> On Tue, Jun 23, 2015 at 7:27 PM, Dylan Baker wrote:
> > I have a couple of python pointers for you, feel free to take them or
> > leave them.
>
> cool, thanks..
>
> What do others think about including shadertoy in shader-db? If it is
> a useful thing, I'll clean up my script and re-submit..
>
> And if we include it, should we commit the scripts we pull down for
> repeatable results and just keep the script as something to resync and
> pull in new shaders? (Esp. given that a small percentage need some
> hand massaging.. I haven't figured out a good way to
> reliably/programatically figure out if the samplers should actually be
> 2d/3d/cube..)
>
I'm in favor of including shadertoy shaders.
-Tom
> That said, I think the shaderdb shaders are a fairly, umm, unique
> stress test of a shader compiler.. and the API to scrape shaders seems
> to convenient to ignore..
>
> BR,
> -R
>
> > Dylan
> >
> > On Tue, Jun 16, 2015 at 03:46:50PM -0400, Rob Clark wrote:
> >> Attached script grabs shaders from shadertoy, and dumps them out as
> >> .shader_test files which can be run through shader-db for compiler
> >> testing.
> >>
> >> shadertoy only gives you a fragment shader (which works based on
> >> gl_FragCoord), so a generic vertex shader is used. And a blurb is
> >> inserted for the pre-defined uniforms and main() function (which just
> >> calls shadertoy mainImage() fxn).
> >>
> >> ---
> >> TODO I guess we'd actually have to parse the shader to figure out if
> >> the sampler uniforms were meant to be 2D/cube/etc. Maybe we just
> >> commit samplers we get from the script and massage them by hand?
> >>
> >> PS. don't make fun of my py too much.. I'm a newb and figuring it
> >> out as I go
> >
> > I'm trying not to make fun, but I do have quite a few pointers for you.
> >
> >>
> >> grab-shadertoy.py | 63
> >> +++
> >> 1 file changed, 63 insertions(+)
> >> create mode 100755 grab-shadertoy.py
> >>
> >> diff --git a/grab-shadertoy.py b/grab-shadertoy.py
> >> new file mode 100755
> >> index 000..74e9d10
> >> --- /dev/null
> >> +++ b/grab-shadertoy.py
> >> @@ -0,0 +1,63 @@
> >> +#!/usr/bin/env python3
> >> +
> >> +
> >> +import requests, json
> >
> > You're not actually using json
> >
> >> +
> >> +url = 'https://www.shadertoy.com/api/v1/shaders'
> >> +key = '?key=NdnKw7'
> >> +
> >> +# Get the list of shaders
> >> +r = requests.get(url + key)
> >> +j = r.json()
> >> +print('Found ' + str(j['Shaders']) + ' shaders')
> >
> > If you use format you can avoid calling str() on everything, and make
> > things more readable using format rather than concatenation:
> > print('Found {} shaders'.format(j['Shaders']))
> >
> >> +
> >> +shader_ids = j['Results']
> >> +for id in shader_ids:
> >> +print('Fetching shader: ' + str(id))
> >> +r = requests.get(url + '/' + id + key)
> >> +j = r.json()
> >> +s = j['Shader']
> >> +info = s['info']
> >> +print('Name: ' + info['name'])
> >> +print('Description: ' + info['description'])
> >> +i = 0;
> >
> > python has a cool builtin called enumerate for doing this:
> > for i, p in enmerate(s['renderpass']):
> >
> > Also, I know it's easy to forget, but python doesn't use ';' at the end
> > of lines, it allows them, but they look weird to pythonistas
> >
> >> +for p in s['renderpass']:
> >> +fobj = open('shaders/shadertoy/' + str(id) + '_' + str(i) +
> >> '.shader_test', 'w')
> >
> > with str.format this would look like:
> > with open('shaders/shadertoy/{}_{}.shader_test'.format(id, i), 'w') as fobj:
> >
> >> +#print('Inputs: ' + str(p['inputs']))
> >> +#print('Outputs: ' + str(p['outputs']))
> >> +fobj.write('[require]\n')
> >> +fobj.write('GLSL >= 1.30\n')
> >> +fobj.write('\n');
> >> +fobj.write('[fragment shader]\n')
> >> +fobj.write('#version 130\n')
> >> +# Shadertoy inserts some uniforms, so we need to do the same:
> >> +fobj.write('uniform vec3 iResolution;\n');
> >> +fobj.write('uniform float iGlobalTime;\n');
> >> +fobj.write('uniform float iChannelTime[4];\n');
> >> +fobj.write('uniform vec4 iMouse;\n');
> >> +fobj.write('uniform vec4 iDate;\n');
> >> +fobj.write('uniform float iSampleRate;\n');
> >> +fobj.write('uniform vec3 iChannelResolution[4];\n');
> >> +# TODO probably need to parse the shader to figure out if
> >> 2d/cubemap/etc
> >> +fobj.write('uniform sampler2D iChannel0;\n');
> >> +fobj.write('uniform sampler2D iChannel1;\n');
> >> +fobj.write('uniform sampler2D iChannel2;\n');
> >> +fobj.write('uniform sampler2D iChannel3;\n');
> >> +# Actual shadertoy shader body:
> >> +fobj.write(p['code'])
> >> +# Shadertoy shader uses mainImage(out vec4 fragColor, in vec2
> >> fragCoord)
> >> +
Re: [Mesa-dev] [PATCH] Fix strict-aliasing violations in GLSL shader list implementation
On 24 June 2015 at 11:44, Ian Romanick wrote:
> On 06/24/2015 03:59 PM, Davin McCall wrote:
>> Hi Ian,
>>
>> On 23/06/15 23:26, Ian Romanick wrote:
>>> On 06/23/2015 02:36 PM, Thomas Helland wrote:
2015-06-24 23:05 GMT+02:00 Davin McCall :
> Hi - I'm new here.
>
> I've recently started poking the Mesa codebase for little reason other
> than
> personal interest. In the "help wanted" section of the website it mentions
> aliasing violations as a target for newcomers to fix, so with that in mind
> I've attached a patch (against git head) which resolves a few of them, by
> targeting the linked list implementation (list.h) used in the GLSL
> compiler/optimizers. This change slightly increases the storage
> requirements
> for a list (adds one word) but resolves the blatant aliasing violation
> that
> was caused by the trick used to conserve that word in the first place.
>
> (I toyed with another approach - using a single sentinel node for both the
> head and tail of a list - but this was much more invasive, and meant that
> you could no longer check whether a particular node was a sentinel node
> unless you had a reference to the list, so I gave up and went with this
> simpler approach).
>
> The most essential change is in the 'exec_list' structure. Three fields
> 'head', 'tail' and 'tail_pred' are removed, and two separate sentinel
> nodes
> are inserted in their place. The old 'head' is replaced by
> 'head_sentinel.next', 'tail_pred' by 'tail_sentinel.prev', and tail
> (always
> NULL) by 'head_sentinel.prev' and 'tail_sentinel.next' (both always NULL).
>>> NAK. The datastructure is correct as-is. It has been in common use
>>> since at least 1985. See the references in the header file.
>>
>> I understand the data structure and how it is supposed to work; the
>> issue is that the trick it employs cannot be implemented in C without
>> breaking the strict aliasing rules (or at least, the current
>> implementation in Mesa breaks the strict aliasing rules). The current
>> code works correctly only with the -fno-strict-aliasing compiler option.
>> The issue is that a pair of 'exec_node *' do not constitute an exec_node
>> in the eyes of the language spec, even though exec_node is declared as
>> holding two such pointers. Consider (from src/glsl/list.h):
>>
>> static inline void
>> exec_list_make_empty(struct exec_list *list)
>> {
>>list->head = (struct exec_node *) & list->tail;
>>list->tail = NULL;
>>list->tail_pred = (struct exec_node *) & list->head;
>> }
>>
>>
>> 'list->head' is of type 'struct exec_node *', and so should point at a
>> 'struct exec_node'. In the code above it is instead co-erced to point
>> at a 'struct exec_node *' (list->tail). That in itself doesn't break the
>> alias rules, but then:
>>
>> static inline bool
>> exec_node_is_tail_sentinel(const struct exec_node *n)
>> {
>>return n->next == NULL;
>> }
>>
>>
>> In 'exec_node_is_tail_sentinel', the sentinel is not actually an
>> exec_node - it is &list->tail. So, if the parameter n does refer to the
>> sentinel, then it does not point to an actual exec_node structure.
>> However, it is de-referenced (by 'n->next') which breaks the strict
>> aliasing rules. This means that the method above can only ever return
>> false, unless it violates the aliasing rules.
>>
>> (The above method could be fixed by casting n to an 'struct exec_node
>> **' and then comparing '**n' against NULL. However, there are other
>> similar examples throughout the code that I do not think would be so
>> trivial).
>>
>> I can quote the relevant parts of the standard if necessary, but your
>> tone somewhat implies that you wouldn't even consider this patch?
>
> Please quote the spec. I have a hard time believing that the compiler
> magically decides that an exec_node* is not really an exec_node* and
> just does whatever it wants (which sounds like what you're describing).
> That sounds pretty rubbish... and I'm surprised that anything works in
> such an environment.
>
> Mesa has also had -fno-strict-aliasing for as long as I can remember,
> and this structure has only been used here for a few years. The whole
> thing just doesn't smell right.
>
Oh we've always had aliasing problems this is just one, you can't
expect one person to fix them all at once.
But making headway is a good thing.
You can't have
struct exec_list *p;
struct exec_node *p2 = (struct exec_list *)p
And do things with p2 and hope that p will get them, because
the compiler wants to store things its doing to p in registers,
and when you go and do something in p2 it can't work out it's the
same thing, so it has to spill/reload.
Dave.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] Fix strict-aliasing violations in GLSL shader list implementation
On 06/23/2015 07:01 PM, Dave Airlie wrote:
> On 24 June 2015 at 11:44, Ian Romanick wrote:
>> On 06/24/2015 03:59 PM, Davin McCall wrote:
>>> Hi Ian,
>>>
>>> On 23/06/15 23:26, Ian Romanick wrote:
On 06/23/2015 02:36 PM, Thomas Helland wrote:
> 2015-06-24 23:05 GMT+02:00 Davin McCall :
>> Hi - I'm new here.
>>
>> I've recently started poking the Mesa codebase for little reason other
>> than
>> personal interest. In the "help wanted" section of the website it
>> mentions
>> aliasing violations as a target for newcomers to fix, so with that in
>> mind
>> I've attached a patch (against git head) which resolves a few of them, by
>> targeting the linked list implementation (list.h) used in the GLSL
>> compiler/optimizers. This change slightly increases the storage
>> requirements
>> for a list (adds one word) but resolves the blatant aliasing violation
>> that
>> was caused by the trick used to conserve that word in the first place.
>>
>> (I toyed with another approach - using a single sentinel node for both
>> the
>> head and tail of a list - but this was much more invasive, and meant that
>> you could no longer check whether a particular node was a sentinel node
>> unless you had a reference to the list, so I gave up and went with this
>> simpler approach).
>>
>> The most essential change is in the 'exec_list' structure. Three fields
>> 'head', 'tail' and 'tail_pred' are removed, and two separate sentinel
>> nodes
>> are inserted in their place. The old 'head' is replaced by
>> 'head_sentinel.next', 'tail_pred' by 'tail_sentinel.prev', and tail
>> (always
>> NULL) by 'head_sentinel.prev' and 'tail_sentinel.next' (both always
>> NULL).
NAK. The datastructure is correct as-is. It has been in common use
since at least 1985. See the references in the header file.
>>>
>>> I understand the data structure and how it is supposed to work; the
>>> issue is that the trick it employs cannot be implemented in C without
>>> breaking the strict aliasing rules (or at least, the current
>>> implementation in Mesa breaks the strict aliasing rules). The current
>>> code works correctly only with the -fno-strict-aliasing compiler option.
>>> The issue is that a pair of 'exec_node *' do not constitute an exec_node
>>> in the eyes of the language spec, even though exec_node is declared as
>>> holding two such pointers. Consider (from src/glsl/list.h):
>>>
>>> static inline void
>>> exec_list_make_empty(struct exec_list *list)
>>> {
>>>list->head = (struct exec_node *) & list->tail;
>>>list->tail = NULL;
>>>list->tail_pred = (struct exec_node *) & list->head;
>>> }
>>>
>>>
>>> 'list->head' is of type 'struct exec_node *', and so should point at a
>>> 'struct exec_node'. In the code above it is instead co-erced to point
>>> at a 'struct exec_node *' (list->tail). That in itself doesn't break the
>>> alias rules, but then:
>>>
>>> static inline bool
>>> exec_node_is_tail_sentinel(const struct exec_node *n)
>>> {
>>>return n->next == NULL;
>>> }
>>>
>>>
>>> In 'exec_node_is_tail_sentinel', the sentinel is not actually an
>>> exec_node - it is &list->tail. So, if the parameter n does refer to the
>>> sentinel, then it does not point to an actual exec_node structure.
>>> However, it is de-referenced (by 'n->next') which breaks the strict
>>> aliasing rules. This means that the method above can only ever return
>>> false, unless it violates the aliasing rules.
>>>
>>> (The above method could be fixed by casting n to an 'struct exec_node
>>> **' and then comparing '**n' against NULL. However, there are other
>>> similar examples throughout the code that I do not think would be so
>>> trivial).
>>>
>>> I can quote the relevant parts of the standard if necessary, but your
>>> tone somewhat implies that you wouldn't even consider this patch?
>>
>> Please quote the spec. I have a hard time believing that the compiler
>> magically decides that an exec_node* is not really an exec_node* and
>> just does whatever it wants (which sounds like what you're describing).
>> That sounds pretty rubbish... and I'm surprised that anything works in
>> such an environment.
>>
>> Mesa has also had -fno-strict-aliasing for as long as I can remember,
>> and this structure has only been used here for a few years. The whole
>> thing just doesn't smell right.
>
> Oh we've always had aliasing problems this is just one, you can't
> expect one person to fix them all at once.
But "With this patch, I can build a working (though perhaps not 100%
bug-free) Mesa without using -fno-strict-aliasing during compilation."
is a pretty strong statement that doesn't completely jibe many years of
Mesa using -fno-strict-aliasing before exec_list was added.
> But making headway is a good thing.
>
> You can't have
> struct exec_list *p;
> struct exec_node *p2
Re: [Mesa-dev] [PATCH] Fix strict-aliasing violations in GLSL shader list implementation
> > Actually, I'm almost 100% certain that there are lots of other strict > aliasing violations in the Mesa code. That's why we've always disabled it. > > More generally, IMO it's unrealistic to rely on strict aliasing for > optimization, because very few people really understand it (I'm not one > of them). I personally think we should get past the, aliasing is hard, lets go shopping, I get a 30K code size reduction on r600_dri.so 6780217 367820 1991392 9139429 8b74e5 lib/gallium/r600_dri.so 6746389 367820 1991392 9105601 8af0c1 lib/gallium/r600_dri.so That isn't a small amount, and I'd think at least for Intel targetting atoms with small caches it might matter. However it might also not be that size once we fixed all the aliasing. Granted there might be other things that would get us that sort of reduction but I'm not sure what they are. Dave. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] configure: use $target_cpu, not $host_cpu when setting asm_arch
On 06/23/2015 04:56 PM, Brian Paul wrote: On 06/23/2015 04:25 PM, Matt Turner wrote: On Tue, Jun 23, 2015 at 3:04 PM, Brian Paul wrote: Otherwise, if we're trying to build a 32-bit Mesa on a 64-bit host we wind up with -DUSE_X86_64_ASM, which is incorrect. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index ddc757e..b12f5f9 100644 --- a/configure.ac +++ b/configure.ac @@ -605,7 +605,7 @@ if test "x$enable_asm" = xyes -a "x$cross_compiling" = xyes; then fi # check for supported arches if test "x$enable_asm" = xyes; then -case "$host_cpu" in +case "$target_cpu" in i?86) case "$host_os" in linux* | *freebsd* | dragonfly* | *netbsd* | openbsd* | gnu*) -- 1.9.1 According to [1], host is "the machine that you are building for" and target is "the machine that GCC will produce code for". In the context of building GCC, I think this means that the resulting GCC binaries will run on $host and will produce code for $target. In the context of Mesa, I can't come up with a way that host != target makes sense. docs/autoconf.html suggests using --build=x86_64-pc-linux-gnu --host=i686-pc-linux-gnu to build on x86_64 for i686. Is that what you're doing? Thanks for the pointer to the docs! I forgot this was mentioned there. I basically had my --host and --build mixed up. Also, I was using "i686-linux-gnu" instead of "i686-pc-linux-gnu" (is there really a difference)? Let me see how far I get now that you set me straight... OK, so the next problem is Mesa is trying to link with 64-bit libdrm instead of the 32-bit one. The command is: libtool: link: g++ -fPIC -DPIC -shared -nostdlib /usr/lib/gcc/x86_64-linux-gnu/4.9/../../../../lib32/crti.o /usr/lib/gcc/x86_64-linux-gnu/4.9/32/crtbeginS.o -Wl,--whole-archive ../../.libs/libmesa.a common/.libs/libmegadriver_stub.a common/.libs/libdricommon.a common/.libs/libxmlconfig.a swrast/.libs/libswrast_dri.a -Wl,--no-whole-archive /usr/lib/x86_64-linux-gnu/libdrm.so -ludev /usr/lib/i386-linux-gnu/libdrm.so /usr/lib/x86_64-linux-gnu/libexpat.so -lpthread -ldl -L/usr/lib/gcc/x86_64-linux-gnu/4.9/32 -L/usr/lib/gcc/x86_64-linux-gnu/4.9/../../../i386-linux-gnu -L/usr/lib/gcc/x86_64-linux-gnu/4.9/../../../../lib32 -L/lib/i386-linux-gnu -L/lib/../lib32 -L/usr/lib/i386-linux-gnu -L/usr/lib/../lib32 -L/usr/lib/gcc/x86_64-linux-gnu/4.9 -L/usr/lib/gcc/x86_64-linux-gnu/4.9/../../.. -lstdc++ -lm -lc -lgcc_s /usr/lib/gcc/x86_64-linux-gnu/4.9/32/crtendS.o /usr/lib/gcc/x86_64-linux-gnu/4.9/../../../../lib32/crtn.o -O0 -m32 -Wl,-Bsymbolic -Wl,--gc-sections -Wl,-soname -Wl,mesa_dri_drivers.so -o .libs/mesa_dri_drivers.so /usr/lib/x86_64-linux-gnu/libdrm.so: error adding symbols: File in wrong format collect2: error: ld returned 1 exit status Note near the middle: /usr/lib/x86_64-linux-gnu/libdrm.so shortly followed by: /usr/lib/i386-linux-gnu/libdrm.so That's after I added PKG_CONFIG_PATH=/usr/lib/i386-linux-gnu/pkgconfig during configuring. Without that, it only tries to link with the 64-bit libdrm. Any ideas? -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] Fix strict-aliasing violations in GLSL shader list implementation
>> And do things with p2 and hope that p will get them, because >> the compiler wants to store things its doing to p in registers, >> and when you go and do something in p2 it can't work out it's the >> same thing, so it has to spill/reload. > > Which I think is different from what Davin was saying, but I may be > misunderstanding the whole thing. That's why I'd like to see spec > language. The part that really gets me is that this is across a > function boundary... that's generally a sacred line, so I'm surprised > that the compiler is allowed to disregard what it's told in that scenario. inlining makes function boundaries nothing, and gcc will really try and inline things. > > I'd also like to see assembly dumps with and without > -fno-strict-aliasing of some place where this goes wrong. If you, > Davin, or someone else can point out a specific function that actually > does the wrong thing, I can generate assembly myself. > > For that matter... how the heck is the ralloc (or any memory allocator) > implementation valid? There are rules on types that can alias, void * and char * are allowed unions are allowed. Dave. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
