Iago Toral <[email protected]> writes: > On Wed, 2015-09-02 at 14:32 +0300, Francisco Jerez wrote: >> Iago Toral <[email protected]> writes: >> >> > On Thu, 2015-07-30 at 16:13 +0300, Francisco Jerez wrote: >> >> Iago Toral <[email protected]> writes: >> >> >> >> > On Thu, 2015-07-30 at 15:58 +0300, Francisco Jerez wrote: >> >> >> Iago Toral Quiroga <[email protected]> writes: >> >> >> >> >> >> > --- >> >> >> > src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp | 2 +- >> >> >> > src/mesa/drivers/dri/i965/brw_vec4.cpp | 2 +- >> >> >> > src/mesa/drivers/dri/i965/intel_debug.c | 3 ++- >> >> >> > src/mesa/drivers/dri/i965/intel_debug.h | 5 +++-- >> >> >> > 4 files changed, 7 insertions(+), 5 deletions(-) >> >> >> > >> >> >> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp >> >> >> > b/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp >> >> >> > index f25f2ec..714248a 100644 >> >> >> > --- a/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp >> >> >> > +++ b/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp >> >> >> > @@ -634,7 +634,7 @@ fs_visitor::assign_regs(bool allow_spilling) >> >> >> > } >> >> >> > >> >> >> > /* Debug of register spilling: Go spill everything. */ >> >> >> > - if (unlikely(INTEL_DEBUG & DEBUG_SPILL)) { >> >> >> > + if (unlikely(INTEL_DEBUG & DEBUG_SPILL_FS)) { >> >> >> > int reg = choose_spill_reg(g); >> >> >> > >> >> >> > if (reg != -1) { >> >> >> > diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp >> >> >> > b/src/mesa/drivers/dri/i965/brw_vec4.cpp >> >> >> > index 53270fb..6cf5ede 100644 >> >> >> > --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp >> >> >> > +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp >> >> >> > @@ -1814,7 +1814,7 @@ vec4_visitor::run(gl_clip_plane *clip_planes) >> >> >> > >> >> >> > setup_payload(); >> >> >> > >> >> >> > - if (false) { >> >> >> > + if (unlikely(INTEL_DEBUG & DEBUG_SPILL_VEC4)) { >> >> >> > /* Debug of register spilling: Go spill everything. */ >> >> >> > const int grf_count = alloc.count; >> >> >> > float spill_costs[alloc.count]; >> >> >> > diff --git a/src/mesa/drivers/dri/i965/intel_debug.c >> >> >> > b/src/mesa/drivers/dri/i965/intel_debug.c >> >> >> > index a077731..8d34349 100644 >> >> >> > --- a/src/mesa/drivers/dri/i965/intel_debug.c >> >> >> > +++ b/src/mesa/drivers/dri/i965/intel_debug.c >> >> >> > @@ -69,7 +69,8 @@ static const struct dri_debug_control >> >> >> > debug_control[] = { >> >> >> > { "ann", DEBUG_ANNOTATION }, >> >> >> > { "no8", DEBUG_NO8 }, >> >> >> > { "vec4vs", DEBUG_VEC4VS }, >> >> >> > - { "spill", DEBUG_SPILL }, >> >> >> > + { "spill_frag", DEBUG_SPILL_FS }, >> >> >> >> >> >> How about we call this "spill_fs" instead? The flag doesn't only >> >> >> affect >> >> >> fragment shaders, AFAICT it will cause all programs compiled with the >> >> >> FS >> >> >> back-end [F for fast ;)] to spill everything. With that fixed: >> >> > >> >> > that was my first choice, but if we do that it seems that >> >> > driParseDebugString will also mark INTEL_DEBUG=fs as enabled. >> >> > >> >> > It seems as if this function checks if any of the string options is >> >> > present in the provided string to enable them, so we can't really use an >> >> > option name where any substring of it is included as a separate >> >> > option :-( >> >> > >> >> >> >> Oh man... That sounds seriously broken... >> > >> > So with that explanation as to why we can't change that, does your Rb >> > stand? >> > >> Seems like a hack and might be confusing because it will cause shaders >> of stages other than fragment to be spilled. But if you insist in using >> a band-aid solution you can have my: >> >> Acked-by: Francisco Jerez <[email protected]> > > It seems that driParseDebugString() does at least notice capitalization, > so what do you think about using "spill_VS" and "spill_FS" instead? > How about we fix it properly? See attachment.
>> >> >> Reviewed-by: Francisco Jerez <[email protected]> >> >> >> >> >> >> > + { "spill_vec4", DEBUG_SPILL_VEC4 }, >> >> >> > { "cs", DEBUG_CS }, >> >> >> > { NULL, 0 } >> >> >> > }; >> >> >> > diff --git a/src/mesa/drivers/dri/i965/intel_debug.h >> >> >> > b/src/mesa/drivers/dri/i965/intel_debug.h >> >> >> > index 4689492..b7d0c82 100644 >> >> >> > --- a/src/mesa/drivers/dri/i965/intel_debug.h >> >> >> > +++ b/src/mesa/drivers/dri/i965/intel_debug.h >> >> >> > @@ -64,8 +64,9 @@ extern uint64_t INTEL_DEBUG; >> >> >> > #define DEBUG_ANNOTATION (1ull << 28) >> >> >> > #define DEBUG_NO8 (1ull << 29) >> >> >> > #define DEBUG_VEC4VS (1ull << 30) >> >> >> > -#define DEBUG_SPILL (1ull << 31) >> >> >> > -#define DEBUG_CS (1ull << 32) >> >> >> > +#define DEBUG_SPILL_FS (1ull << 31) >> >> >> > +#define DEBUG_SPILL_VEC4 (1ull << 32) >> >> >> > +#define DEBUG_CS (1ull << 33) >> >> >> > >> >> >> > #ifdef HAVE_ANDROID_PLATFORM >> >> >> > #define LOG_TAG "INTEL-MESA" >> >> >> > -- >> >> >> > 1.9.1
From 287c8e88889486c030e9e1c89b93f26fb700ed76 Mon Sep 17 00:00:00 2001 From: Francisco Jerez <[email protected]> Date: Thu, 3 Sep 2015 14:50:12 +0300 Subject: [PATCH 1/2] dri/common: Fix codestyle of driParseDebugString(). --- src/mesa/drivers/dri/common/utils.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/mesa/drivers/dri/common/utils.c b/src/mesa/drivers/dri/common/utils.c index 43d90d9..1e3b15b 100644 --- a/src/mesa/drivers/dri/common/utils.c +++ b/src/mesa/drivers/dri/common/utils.c @@ -43,19 +43,17 @@ uint64_t -driParseDebugString( const char * debug, - const struct dri_debug_control * control ) +driParseDebugString(const char *debug, + const struct dri_debug_control *control) { uint64_t flag = 0; - if ( debug != NULL ) { - while( control->string != NULL ) { - if ( !strcmp( debug, "all" ) || - strstr( debug, control->string ) != NULL ) { + if (debug != NULL) { + for (; control->string != NULL; control++) { + if (!strcmp(debug, "all") || + strstr(debug, control->string) != NULL) { flag |= control->flag; } - - control++; } } -- 2.4.6
From 9b868581bf23247c30878dd6044a6cde9999143e Mon Sep 17 00:00:00 2001 From: Francisco Jerez <[email protected]> Date: Thu, 3 Sep 2015 15:20:04 +0300 Subject: [PATCH 2/2] dri/common: Tokenize driParseDebugString() argument before matching debug flags. Fixes debug string parsing when one of the supported flags is a substring of another. --- src/mesa/drivers/dri/common/utils.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/mesa/drivers/dri/common/utils.c b/src/mesa/drivers/dri/common/utils.c index 1e3b15b..1246bec 100644 --- a/src/mesa/drivers/dri/common/utils.c +++ b/src/mesa/drivers/dri/common/utils.c @@ -50,10 +50,19 @@ driParseDebugString(const char *debug, if (debug != NULL) { for (; control->string != NULL; control++) { - if (!strcmp(debug, "all") || - strstr(debug, control->string) != NULL) { - flag |= control->flag; - } + if (!strcmp(debug, "all")) { + flag |= control->flag; + + } else { + const char *s = debug; + unsigned n; + + for (; n = strcspn(s, ", "), *s; s += MAX2(1, n)) { + if (strlen(control->string) == n && + !strncmp(control->string, s, n)) + flag |= control->flag; + } + } } } -- 2.4.6
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
