[Mesa-dev] [Mesa-stable] [PATCH] clover: Fix build against LLVM 3.8 SVN >= r255078

[Emil Velikov]

On 15 December 2015 at 09:04, Michel Dänzer <michel at daenzer.net> wrote:
> On 15.12.2015 09:17, Ilia Mirkin wrote:
>> On Wed, Dec 9, 2015 at 5:30 AM, Francisco Jerez <currojerez at riseup.net> wrote:
>>> Michel Dänzer <michel at daenzer.net> writes:
>>>
>>>> From: Michel Dänzer <michel.daenzer at amd.com>
>>>>
>>>> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>
>>>
>>> Looks OK to me,
>>> Reviewed-by: Francisco Jerez <currojerez at riseup.net>
>>>
>>>> ---
>>>>  src/gallium/state_trackers/clover/llvm/invocation.cpp | 4 ++++
>>>>  1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp b/src/gallium/state_trackers/clover/llvm/invocation.cpp
>>>> index 3b37f08..4d11c24 100644
>>>> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
>>>> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
>>>> @@ -661,7 +661,11 @@ namespace {
>>>>
>>>>        if (dump_asm) {
>>>>           LLVMSetTargetMachineAsmVerbosity(tm, true);
>>>> +#if HAVE_LLVM >= 0x0308
>>>> +         LLVMModuleRef debug_mod = wrap(llvm::CloneModule(mod).release());
>>>> +#else
>>>>           LLVMModuleRef debug_mod = wrap(llvm::CloneModule(mod));
>>>> +#endif
>>>>           emit_code(tm, debug_mod, LLVMAssemblyFile, &out_buffer, r_log);
>>>>           buffer_size = LLVMGetBufferSize(out_buffer);
>>>>           buffer_data = LLVMGetBufferStart(out_buffer);
>>
>> Emil, consider cherry-picking this into 11.1 and perhaps even 11.0 to
>> save people from unnecessary compilation trouble. This is commit
>> b4a03e7f8f upstream.
>
> FWIW, I still think that's a bad idea at this point: Supporting
> unreleased snapshots of LLVM simply isn't feasible on stable Mesa
> branches — the next similar breakage can appear in LLVM SVN anytime.
>
> To help people running into this, maybe stable Mesa branches could get a
> change to configure.ac which aborts with a descriptive error message
> when trying to build against a version of LLVM which isn't supported on
> that stable Mesa branch yet.
>
I second Michel here. Also let's not forget that getting things to
build does not always mean "runs without crashing horribly".

On the actual checking at hand... that only thing that comes to mind
is using llvm_check_version_for(), everywhere throughout configure.ac
and changing it roughly like

if test "${LLVM_VERSION_INT}${LLVM_VERSION_PATCH}" -lt "${1}0${2}${3}" -a
   test "${LLVM_VERSION_INT}${LLVM_VERSION_PATCH}" -gt
"${LLVM_MAX_VERSION_SUPPORTED}"; then
    AC_MSG_ERROR([Version LLVM $1.$2.$3 up-to
${LLVM_MAX_VERSION_SUPPORTED} is required for $4])
fi

We can have this everywhere or just in stable. Any the person who
wants the restriction lifted (in the stable branches) should provide
some "evidence" that llvmpipe, clover and radeons aren't horribly
broken ? "Evidence" might be an overkill although an ack from the
people working in those areas feels like a must have imho.

Thoughts, suggestions, objections ?
-Emil