On 30 June 2018 at 13:30, Marek Olšák <mar...@gmail.com> wrote:
> On Tue, Jun 26, 2018 at 11:58 PM, Dave Airlie <airl...@gmail.com> wrote:
>> From: Dave Airlie <airl...@redhat.com>
>>
>> This creates a common per-thread compiler info struct, and adds
>> the init code to it. This is mostly ported from radeonsi.
>>
>> The common info struct is used in radv first and replaces the
>> current code.
>> ---
>> src/amd/common/ac_llvm_util.c | 50 +++++++++++++++++++++++++++++++
>> src/amd/common/ac_llvm_util.h | 14 +++++++++
>> src/amd/vulkan/radv_nir_to_llvm.c | 39 ++++++++++--------------
>> src/amd/vulkan/radv_private.h | 7 ++---
>> src/amd/vulkan/radv_shader.c | 16 +++++-----
>> 5 files changed, 91 insertions(+), 35 deletions(-)
>>
>> diff --git a/src/amd/common/ac_llvm_util.c b/src/amd/common/ac_llvm_util.c
>> index dd2469d4606..85dc9d72a5c 100644
>> --- a/src/amd/common/ac_llvm_util.c
>> +++ b/src/amd/common/ac_llvm_util.c
>> @@ -188,6 +188,56 @@ LLVMPassManagerRef
>> ac_init_passmgr(LLVMTargetLibraryInfoRef target_library_info,
>> return passmgr;
>> }
>>
>> +bool ac_llvm_compiler_init(struct ac_llvm_compiler_info *info,
>> + bool add_target_library_info,
>> + enum radeon_family family,
>> + enum ac_target_machine_options tm_options)
>> +{
>> + memset(info, 0, sizeof(*info));
>> + info->tm = ac_create_target_machine(family, tm_options,
>> &info->triple);
>> + if (!info->tm)
>> + return false;
>> +
>> + /* Get the data layout. */
>> + LLVMTargetDataRef data_layout = LLVMCreateTargetDataLayout(info->tm);
>> + if (!data_layout)
>> + goto fail;
>> + info->data_layout = LLVMCopyStringRepOfTargetData(data_layout);
>> + LLVMDisposeTargetData(data_layout);
>> +
>> +#if HAVE_LLVM < 0x0700
>
> This #if is not needed. You already have a bool flag coming from radv.
> You can modify the bool value in radv.
>
>> + if (add_target_library_info)
>> +#endif
The if flag is to encapsulate things in this file, if llvm target
library info is broken,
and we have to leak something, then the driver indicates if can deal with the
leak or not with the flag. If the leak isn't going to happen then
we've encapsulate
the leak problem in one place.
Having the HAVE_LLVM in the driver to denote that LLVM is broken seems wrong,
granted not having broken llvm in the first place would have been nice.
I'll think about if I can make it clearer at least why this is here.
Dave.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
