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

Vicki Pfau vi at endrift.com
Fri Aug 24 16:22:36 UTC 2018


Is there anything else I need to do on this? This is my first mesa patch 
so I'm not entirely clear what next steps are for getting it committed.


On 08/20/2018 02:44 PM, Roland Scheidegger wrote:
> Alright, I guess it's ok then.
> In theory the u_cpu_detect bits could be used in different places, for
> instance the translate code emits its own sse code, and as long as a
> feature was detected properly it may make sense to disable it only for
> some users. Albeit llvm setup and the gallivm code need to agree
> generally, and there's no good way to deal with this right now (I
> suppose gallivm actually should use its own copy of the u_cpu bits). The
> fiddling we do in lp_bld_init() wrt SSE (LP_FORCE_SSE2 and also avx
> disabling) isn't a clean way neither.
> So this looks like as good a solution as others.
>
> Reviewed-by: Roland Scheidegger <sroland at vmware.com>
>
> Am 20.08.2018 um 22:15 schrieb 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;
>>>>   



More information about the mesa-dev mailing list