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

[Jose Fonseca]

On 17/11/15 15:15, Oded Gabbay wrote:
> On Tue, Nov 17, 2015 at 4:42 PM, Roland Scheidegger <sroland at vmware.com> wrote:
>> Am 17.11.2015 um 15:19 schrieb Oded Gabbay:
>>> 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.
>>>
>>> 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 | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>>> index 7bda118..8c74cb8 100644
>>> --- a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>>> +++ b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>>> @@ -536,6 +536,7 @@ lp_build_create_jit_compiler_for_module(LLVMExecutionEngineRef *OutJIT,
>>>
>>>   #if defined(PIPE_ARCH_PPC)
>>>      MAttrs.push_back(util_cpu_caps.has_altivec ? "+altivec" : "-altivec");
>>> +   MAttrs.push_back("-vsx");
>>>   #endif
>>>
>>>      builder.setMAttrs(MAttrs);
>>>
>>
>> Doesn't that need some llvm version check, otherwise the attribute might
>> be unsupported (not entirely sure what happens in this case)? From a
>> very quick look, vsx seems to be a fairly recent addition (that is,
>> might not be in llvm 3.3 which is the minimum, at least for x86).
>> Otherwise looks ok to me. Albeit if llvm indeed miscompiles this ought
>> to be fixed as that's quite nasty (we had some bugs in the past where we
>> thought it might be miscompilation and it was due to us relying on
>> undefined behavior too).
>>
>> Roland
>>
>
> Hi Roland,
> vsx was added in release 3.4, according to git log. I believe there is
> no need for version check because I don't think ppc64le distributions
> uses llvm lower than that (I know that in Red Hat/Fedora we have at
> least 3.4). But if you insist I can add that check.

>
> I'm in contact with LLVM devs from IBM to solve this bug and from my
> debugging (and I did a LOT of it), I don't believe its our misuse of
> the code. Based on different experiments I did (remove optimizations,
> replace vmx with vsx instructions and more), I truly think this is a
> bug in LLVM backend. It is not an easy bug - you need to go over each
> vsx instruction and make sure if its lane-sensitive or not. If so, you
> need to add a special treatment inside LLVM code for it.
>
> Once I get a resolution from IBM's devs, I will of course remove this
> workaround. Moreover, once that happens, I will probably move some of
> the vmx calls to vsx, as vsx have a larger register file.
>
>      Oded


Let's add the LLVM PR URL in a commment above the line for future reference.

Jose