Iago Toral <ito...@igalia.com> writes:
> On Thu, 2015-09-03 at 15:37 +0300, Francisco Jerez wrote:
>> Iago Toral <ito...@igalia.com> writes:
>>
>> > On Wed, 2015-09-02 at 14:32 +0300, Francisco Jerez wrote:
>> >> Iago Toral <ito...@igalia.com> writes:
>> >>
>> >> > On Thu, 2015-07-30 at 16:13 +0300, Francisco Jerez wrote:
>> >> >> Iago Toral <ito...@igalia.com> writes:
>> >> >>
>> >> >> > On Thu, 2015-07-30 at 15:58 +0300, Francisco Jerez wrote:
>> >> >> >> Iago Toral Quiroga <ito...@igalia.com> 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 <curroje...@riseup.net>
>> >
>> > 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.
>
> Both patches are
> Reviewed-by: Iago Toral Quiroga <ito...@igalia.com>
>
> I'll push these two together with the original version of this patch
> that uses spill_vec4 and spill_fs for the debug options.
>
> Iago
>
Thanks!
>> >> >> >> Reviewed-by: Francisco Jerez <curroje...@riseup.net>
>> >> >> >>
>> >> >> >> > + { "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
>>
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
