Am 24.08.2018 um 18:22 schrieb Vicki Pfau: > 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. No that's all needed, pushed, sorry for the delay. Thanks!
Roland > > > 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 <srol...@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 <v...@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; >>>>> > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev