[Mesa-dev] [PATCH shader-db] Set more env-var's in run.c

[Rob Clark]

On Tue, Dec 1, 2015 at 8:00 PM, Matt Turner <mattst88 at gmail.com> wrote:
> On Tue, Dec 1, 2015 at 4:40 PM, Rob Clark <robdclark at gmail.com> wrote:
>> Simplify the instructions a bit by setting all the potentially needed
>> environment variables in run.c itself.
>>
>> Made very trivially more complex by the fact that all of the gallium
>> drivers are using u_debug stuff, so one env var contains potentially
>> multiple flags.  We don't want to clobber any other flags that are set
>> so we simply append.
>> ---
>>  README |  4 ++--
>>  run.c  | 22 ++++++++++++++++++++--
>>  2 files changed, 22 insertions(+), 4 deletions(-)
>>
>> diff --git a/README b/README
>> index 5477bf5..06294c9 100644
>> --- a/README
>> +++ b/README
>> @@ -37,7 +37,7 @@ to run.
>>
>>  === Running shaders ===
>>
>> -ST_DEBUG=precompile R600_DEBUG=ps,vs,gs,precompile ./run shaders -1 2> new-run
>> +./run shaders -1 2> new-run
>>
>>  Note that a debug mesa build required (ie. --enable-debug)
>>
>> @@ -50,7 +50,7 @@ Note that a debug mesa build required (ie. --enable-debug)
>>
>>  === Running shaders ===
>>
>> -FD_MESA_DEBUG=shaderdb ST_DEBUG=precompile ./run -1 shaders 2> new-run
>> +./run -1 shaders 2> new-run
>
> Huh, I didn't realize freedreno support was a thing. Must have missed the patch.

New..  or, well, it has existed for a while but didn't get around to
writing analyse script and pushing it until today.

>>
>>  Note that a debug mesa build required (ie. --enable-debug)
>
> That's awful... at least for a lot of shaders. You're going to want to
> run shader-db on optimized builds.

I don't disagree..  that is just the current state on the mesa side.
And it took me longer than I'd care to admit to realize the reason
things weren't working was that I had a non-debug build ;-)

This is also why I left the './run -1' bits in the README rather than
trying to detect driver and decide (since I don't think I can sanely
detect driver version for when we have a better solution)

>>
>> diff --git a/run.c b/run.c
>> index 73e468d..82d8c91 100644
>> --- a/run.c
>> +++ b/run.c
>> @@ -23,13 +23,15 @@
>>   * DEALINGS IN THE SOFTWARE.
>>   */
>>
>> +/* for memmem(). The man page doesn't say __USE_GNU... */
>> +/* also for asprintf() */
>> +#define _GNU_SOURCE
>> +
>>  #include <time.h>
>>  #include <stdio.h>
>>  #include <fcntl.h>
>>  #include <assert.h>
>>  #include <signal.h>
>> -/* for memmem(). The man page doesn't say __USE_GNU... */
>> -#define __USE_GNU
>>  #include <string.h>
>>  #include <stdlib.h>
>>  #include <unistd.h>
>> @@ -292,6 +294,19 @@ void print_usage(const char *prog_name)
>>              prog_name);
>>  }
>>
>> +static void addenv(const char *name, const char *value)
>> +{
>> +    const char *orig = getenv(name);
>> +    if (orig) {
>> +        char *newval;
>> +        asprintf(&newval, "%s,%s", orig, value);
>> +        setenv(name, newval, 1);
>> +        free(newval);
>> +    } else {
>> +        setenv(name, value, 1);
>> +    }
>> +}
>> +
>>  int
>>  main(int argc, char **argv)
>>  {
>> @@ -340,6 +355,9 @@ main(int argc, char **argv)
>>
>>      setenv("allow_glsl_extension_directive_midshader", "true", 1);
>>      setenv("shader_precompile", "true", 1);
>> +    addenv("ST_DEBUG", "precompile");
>> +    addenv("R600_DEBUG", "ps,vs,gs,precompile");
>> +    addenv("FD_MESA_DEBUG", "shaderdb");
>
> Other than allow_glsl_extension_directive_midshader, I don't think any
> of these should really be necessary ideally. I think adding
> shader_precompile was unnecessary since it's on by default.

ST_DEBUG=precompile is defn needed.. otherwise *something* (not sure
if it is core or mesa-st) decides to not compile things yet..

We should, IMO, split that out out into different env var, or make
ST_DEBUG (partially?) supported for non-debug builds.  The need for
debug build is a problem.

> The Radeon support is hacky and predates support for getting KHR_debug
> messages through all the Gallium layers. I'd kinda rather leave those
> envars in the README as a reminder to use KHR_debug. :)
>
> And why isn't freedreno using KHR_debug?

Only because the mesa-side freedreno support has been around for a
while and pre-dates the KHR_debug stuff (and was based on the way the
radeon worked, since I think at the time it and vc4 where the only
other gallium drivers supporting shader-db)..

I actually just found out about the KHR_debug stuff a bit earlier on
IRC..  at some point plan to move to that.  But I think we can keep
the env var's in run.c since it is useful to actually be able to
compare old/new mesa versions, and if they are later unneeded a few
extra setenv() calls isn't going to hurt anything.

> I don't get it. I write this tool that allows parallel compilation and
> everyone just hacks up their drivers (and now shader-db) enough to get
> non-concurrent compilation.

I guess it isn't such a big deal with the relatively smaller public
shader-db shader collection ;-)

Don't get me wrong, it would be nice to fix things so we could use
parallel..  but some fires are bigger than others.

BR,
-R