[Mesa-dev] [PATCH] gallivm: Detect VSX separately from Altivec

[Vicki Pfau]

I was mostly following what was done earlier in the file for Altivec. I 
can move it but then ideally the Alitvec check should also be moved.


Vicki


On 08/20/2018 08:53 AM, Roland Scheidegger wrote:
> u_cpu_detect should detect what's really available, not what is used
> (though indeed we actually disable u_cpu bits explicitly in gallivm for
> some sse features, but this is a hack).
> So I think it would be better if u_cpu_detect sets the has_vsx bit
> regardless what the env var is and then enable it based on this bit and
> the env var.
> Otherwise looks good to me.
>
> Roland
>
> Am 19.08.2018 um 23:17 schrieb Vicki Pfau:
>> Previously gallivm would attempt to use VSX instructions on all systems
>> where it detected that Altivec is supported; however, VSX was added to
>> POWER long after Altivec, causing lots of crashes on older POWER/PPC
>> hardware, e.g. PPC Macs. By detecting VSX separately from Altivec we can
>> automatically disable it on hardware that supports Altivec but not VSX
>>
>> Signed-off-by: Vicki Pfau <vi at endrift.com>
>> ---
>>   src/gallium/auxiliary/gallivm/lp_bld_misc.cpp | 21 +++----------------
>>   src/gallium/auxiliary/util/u_cpu_detect.c     | 14 ++++++++++++-
>>   src/gallium/auxiliary/util/u_cpu_detect.h     |  1 +
>>   3 files changed, 17 insertions(+), 19 deletions(-)
>>
>> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>> index 79dbedbb56..fcbdd5050f 100644
>> --- a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>> +++ b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>> @@ -650,26 +650,11 @@ lp_build_create_jit_compiler_for_module(LLVMExecutionEngineRef *OutJIT,
>>       * which are fixed in LLVM 4.0.
>>       *
>>       * With LLVM 4.0 or higher:
>> -    * Make sure VSX instructions are ENABLED, unless
>> -    * a) the entire -mattr option is overridden via GALLIVM_MATTRS, or
>> -    * b) VSX instructions are explicitly enabled/disabled via GALLIVM_VSX=1 or 0.
>> +    * Make sure VSX instructions are ENABLED (if supported), unless
>> +    * VSX instructions are explicitly enabled/disabled via GALLIVM_VSX=1 or 0.
>>       */
>>      if (util_cpu_caps.has_altivec) {
>> -      char *env_mattrs = getenv("GALLIVM_MATTRS");
>> -      if (env_mattrs) {
>> -         MAttrs.push_back(env_mattrs);
>> -      }
>> -      else {
>> -         boolean enable_vsx = true;
>> -         char *env_vsx = getenv("GALLIVM_VSX");
>> -         if (env_vsx && env_vsx[0] == '0') {
>> -            enable_vsx = false;
>> -         }
>> -         if (enable_vsx)
>> -            MAttrs.push_back("+vsx");
>> -         else
>> -            MAttrs.push_back("-vsx");
>> -      }
>> +      MAttrs.push_back(util_cpu_caps.has_vsx ? "+vsx" : "-vsx");
>>      }
>>   #endif
>>   #endif
>> diff --git a/src/gallium/auxiliary/util/u_cpu_detect.c b/src/gallium/auxiliary/util/u_cpu_detect.c
>> index 3c6ae4ea1a..14003aa769 100644
>> --- a/src/gallium/auxiliary/util/u_cpu_detect.c
>> +++ b/src/gallium/auxiliary/util/u_cpu_detect.c
>> @@ -133,6 +133,7 @@ check_os_altivec_support(void)
>>         signal(SIGILL, SIG_DFL);
>>      } else {
>>         boolean enable_altivec = TRUE;    /* Default: enable  if available, and if not overridden */
>> +      boolean enable_vsx = TRUE;
>>   #ifdef DEBUG
>>         /* Disabling Altivec code generation is not the same as disabling VSX code generation,
>>          * which can be done simply by passing -mattr=-vsx to the LLVM compiler; cf.
>> @@ -144,6 +145,11 @@ check_os_altivec_support(void)
>>            enable_altivec = FALSE;
>>         }
>>   #endif
>> +      /* VSX instructions can be explicitly enabled/disabled via GALLIVM_VSX=1 or 0 */
>> +      char *env_vsx = getenv("GALLIVM_VSX");
>> +      if (env_vsx && env_vsx[0] == '0') {
>> +         enable_vsx = FALSE;
>> +      }
>>         if (enable_altivec) {
>>            __lv_powerpc_canjump = 1;
>>   
>> @@ -153,8 +159,13 @@ check_os_altivec_support(void)
>>                :
>>                : "r" (-1));
>>   
>> -         signal(SIGILL, SIG_DFL);
>>            util_cpu_caps.has_altivec = 1;
>> +
>> +         if (enable_vsx) {
>> +            __asm __volatile("xxland %vs0, %vs0, %vs0");
>> +            util_cpu_caps.has_vsx = 1;
>> +         }
>> +         signal(SIGILL, SIG_DFL);
>>         } else {
>>            util_cpu_caps.has_altivec = 0;
>>         }
>> @@ -536,6 +547,7 @@ util_cpu_detect(void)
>>         debug_printf("util_cpu_caps.has_3dnow_ext = %u\n", util_cpu_caps.has_3dnow_ext);
>>         debug_printf("util_cpu_caps.has_xop = %u\n", util_cpu_caps.has_xop);
>>         debug_printf("util_cpu_caps.has_altivec = %u\n", util_cpu_caps.has_altivec);
>> +      debug_printf("util_cpu_caps.has_vsx = %u\n", util_cpu_caps.has_vsx);
>>         debug_printf("util_cpu_caps.has_neon = %u\n", util_cpu_caps.has_neon);
>>         debug_printf("util_cpu_caps.has_daz = %u\n", util_cpu_caps.has_daz);
>>         debug_printf("util_cpu_caps.has_avx512f = %u\n", util_cpu_caps.has_avx512f);
>> diff --git a/src/gallium/auxiliary/util/u_cpu_detect.h b/src/gallium/auxiliary/util/u_cpu_detect.h
>> index 7a63d55028..19f5567ca7 100644
>> --- a/src/gallium/auxiliary/util/u_cpu_detect.h
>> +++ b/src/gallium/auxiliary/util/u_cpu_detect.h
>> @@ -71,6 +71,7 @@ struct util_cpu_caps {
>>      unsigned has_3dnow_ext:1;
>>      unsigned has_xop:1;
>>      unsigned has_altivec:1;
>> +   unsigned has_vsx:1;
>>      unsigned has_daz:1;
>>      unsigned has_neon:1;
>>   
>>