[Mesa-dev] [PATCH v3 25/25] configure.ac: Add required LLVM versions to the top

[Emil Velikov]

On 14 October 2016 at 09:45, Michel Dänzer <michel at daenzer.net> wrote:
> On 14/10/16 05:14 PM, Emil Velikov wrote:
>> On 14 October 2016 at 01:45, Michel Dänzer <michel at daenzer.net> wrote:
>>> On 13/10/16 07:14 PM, Emil Velikov wrote:
>>>> On 13 October 2016 at 04:07, Michel Dänzer <michel at daenzer.net> wrote:
>>>>> On 13/10/16 03:37 AM, Tobias Droste wrote:
>>>>>> Am Mittwoch, 12. Oktober 2016, 11:53:50 CEST schrieb Emil Velikov:
>>>>>>>>
>>>>>>>> +LLVM_VERSION_REQUIRED_OPENCL=3.6.0
>>>>>>>> +LLVM_VERSION_REQUIRED_R600=3.6.0
>>>>>>>> +LLVM_VERSION_REQUIRED_RADEONSI=3.6.0
>>>>>>>
>>>>>>> There's a small related gotcha: as-is at build time we get the
>>>>>>> different codepaths thus, as people build against shared LLVM (hello
>>>>>>> Archlinux, I'm looking at you) and update their LLVM without
>>>>>>> rebuilding mesa (Arch I'm looking at you again) things go funny.
>>>>>
>>>>> What exactly happened there? LLVM upstream generates shared libraries
>>>>> named libLLVM-<major>.<minor>.so*, so it shouldn't be possible for a
>>>>> simple LLVM package update to break Mesa, unless Arch did something
>>>>> really stupid.
>>>>>
>>>> The issue is not specific to Arch, but anyone who links against shared LLVM.
>>>>
>>>> Here is another take on it:
>>>> At least swr and r600/radeonsi depend at _build_ time on the _patch_
>>>> version of LLVM. The latter of which is not part of the DSO name. Thus
>>>> at runtime as you change your LLVM (3.9.x>3.9.y) you'll execute the
>>>> "3.9.x" codepath even though you are be using "3.9.y" LLVM.
>>>
>>> That should be fine, since 3.9.y is backwards compatible with 3.9.x.
>>>
>>> Debian doesn't automatically recompile Mesa in such cases either, and I
>>> haven't seen any problems there because of that.
>>>
>>> So, what exactly was the problem?
>>>
>> Just grep through for LLVM_.*PATCH and you'll see it. Portable code
>> should not check that at compile time.
>
> This is getting a bit annoying... Please explicitly say what you think
> is a problem, especially after being asked to do so multiple times.
>
AFAICT picking on like an old bat can be annoying, so I've tried to avoid it.
Regardless, as per your request:

* src/gallium/drivers/radeon/r600_pipe_common.c
No actual bug, yet misleading.

* src/gallium/drivers/radeonsi/si_pipe.c
Update to 3.6.2+, still missing tessellation unless you rebuild mesa.
The latter bug in itself.
Downgrade - TBD, depending on the (fixed) LLVM bugs.

* src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
Analogous to above - s/missing tessellation/VI+ chips not working/.

Reading back the wrong version (as per r600_pipe_common.c) combined
with tess bugs/missing VI+ support leads to some wtf moments, since
the rebuild requirement isn't obvious.

src/gallium/drivers/swr/rasterizer/jitter/scripts/gen_llvm_ir_macros.py
Reversed argument order. No issues if the whole things gets inlined
into mesa, fun experience otherwise.

Using a runtime check for shared libs and compile/run-time one for
static ones the "better" choice, imho. That or just bump the
requirement ?

Just pointing out what seems odd/wrong, if I'm the only one so be it :-\

Thanks
Emil