[Mesa-dev] [Mesa-stable] [PATCH v2] llvmpipe: disable vsx in ppc due to LLVM PPC bug

[Oded Gabbay]

On Tue, Nov 17, 2015 at 9:40 PM, Jan Vesely <jan.vesely at rutgers.edu> wrote:
>
>
> On Tue, Nov 17, 2015 at 12:37 PM, Oded Gabbay <oded.gabbay at gmail.com> wrote:
>>
>> On Tue, Nov 17, 2015 at 6:15 PM, Emil Velikov <emil.l.velikov at gmail.com>
>> wrote:
>> > On 17 November 2015 at 16:02, Oded Gabbay <oded.gabbay at gmail.com> wrote:
>> >> This patch makes sure that if we use altivec (VMX) instructions, we
>> >> don't
>> >> use VSX instructions as well, as this cause piglit tests to fail
>> >>
>> >> For more details, see: https://llvm.org/bugs/show_bug.cgi?id=25503#c7
>> >>
>> >> With this patch, ppc64le reaches parity with x86-64 as far as piglit
>> >> test
>> >> suite is concerned.
>> >>
>> >> v2:
>> >> - Added check that we have at least LLVM 3.4
>> >> - Added the LLVM bug URL as a comment in the code
>> >>
>> >> Signed-off-by: Oded Gabbay <oded.gabbay at gmail.com>
>> >> Cc: "11.0" <mesa-stable at lists.freedesktop.org>
>> >> ---
>> >>  src/gallium/auxiliary/gallivm/lp_bld_misc.cpp | 4 ++++
>> >>  1 file changed, 4 insertions(+)
>> >>
>> >> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>> >> b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>> >> index 7bda118..152593a 100644
>> >> --- a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>> >> +++ b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>> >> @@ -536,6 +536,10 @@
>> >> lp_build_create_jit_compiler_for_module(LLVMExecutionEngineRef *OutJIT,
>> >>
>> >>  #if defined(PIPE_ARCH_PPC)
>> >>     MAttrs.push_back(util_cpu_caps.has_altivec ? "+altivec" :
>> >> "-altivec");
>> >> +#if HAVE_LLVM >= 0x0304
>> >> +   /* See LLVM bug https://llvm.org/bugs/show_bug.cgi?id=25503#c7 */
>> >> +   MAttrs.push_back("-vsx");
>> > The commit message does not reflect what the patch actually does. I
>> > cannot object against the patch in any way, although the two should be
>> > in sync imho.
>> >
>> > Base of a very quick look at the llvm bug, I'm leaning that the commit
>> > msg is correct and the patch is off ?
>> >
>> > Thanks
>> > Emil
>>
>> Hmm, I'm not sure I understand what you mean.
>>
>> The commit message says: "This patch makes sure that if we use altivec
>> (VMX) instructions, we don't
>> use VSX instructions as well, as this cause piglit tests to fail"
>>
>> And the patch itself disables the VSX attribute in LLVM backend in
>> case we use Altivec (VMX) - meaning that no VSX instructions will be
>> generated alongside Altivec instructions.
>>
>> So unless I completely misunderstood something, the commit message and
>> the patch match.
>
>
> sorry to meddle. the commit message suggests that you only want to disable
> vsx if altivec is enabled. However, the patch adds -vsx unconditionally. Do
> you want to ever have "-altivec -vsx"?
>
> Jan

There is no option, AFAIK, that altivec support is missing, but vsx
support exists. So, if we write "-altivec", then "-vsx" has no
meaning, cause there isn't vsx support anyway.
However, just to make it logically correct, I will send another
version that only disables vsx if altivec is enabled

       Oded

>
>>
>>
>> The llvm bug description maybe a little misleading, because I started
>> it last week and then I had a different impression. But that's why I
>> pointed to comment #7 which is where I wrote the updated description,
>> which matches this patch (workaround).
>>
>>       Oded
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>