[Mesa-dev] [PATCH] ac/nir/llvm: Fix setting function attributes for intrinsics

[Bas Nieuwenhuizen]

On Fri, Nov 11, 2016 at 10:35 PM, Daniel Scharrer <daniel at constexpr.org> wrote:
> On 2016-11-11 22:24, Bas Nieuwenhuizen wrote:
>> Maybe also add an assert in ac_add_function_attr that the attr is a
>> single flag? Something like
>>
>> assert(attr && util_is_power_of_two(attr));
>
> There is already a warning printed for unknown attributes in
> ac_attr_to_llvm_attr / attr_to_str that catches this.
>
>> Either way, this patch is
>> Reviewed-by: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
>
> Thanks. I don't have commit access so I'll need someone else to push it.

Pushed.

>
>> On Fri, Nov 11, 2016 at 9:36 PM, Daniel Scharrer <daniel at constexpr.org> wrote:
>>> This fixes a NULL pointer dereference for intrinsics with more than
>>> one function attribute introduced in commit 2fdaf38.
>>> The fix is ported from the lp_build_intrinsic changes in commit 8bdd52c.
>>> ---
>>>
>>> I'm a bit unsure about the index change from 0 to -1, but the gallium code does
>>> this as well and without it all vulkan apps fail with:
>>>  LLVM ERROR: Cannot select: target intrinsic %llvm.SI.vs.load.input
>>>
>>>  src/amd/common/ac_nir_to_llvm.c | 6 +++++-
>>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
>>> index b7ccea8..aa2d488 100644
>>> --- a/src/amd/common/ac_nir_to_llvm.c
>>> +++ b/src/amd/common/ac_nir_to_llvm.c
>>> @@ -1675,7 +1675,11 @@ emit_llvm_intrinsic(struct nir_to_llvm_context *ctx, const char *name,
>>>                 LLVMSetFunctionCallConv(function, LLVMCCallConv);
>>>                 LLVMSetLinkage(function, LLVMExternalLinkage);
>>>
>>> -               ac_add_function_attr(function, 0, attrib_mask | AC_FUNC_ATTR_NOUNWIND);
>>> +               attrib_mask |= AC_FUNC_ATTR_NOUNWIND;
>>> +               while (attrib_mask) {
>>> +                       enum ac_func_attr attr = 1u << u_bit_scan(&attrib_mask);
>>> +                       ac_add_function_attr(function, -1, attr);
>>> +               }
>>>         }
>>>         return LLVMBuildCall(ctx->builder, function, params, param_count, "");
>>>  }
>>> --
>>> 2.10.2
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
> --
> Daniel Scharrer
> http://constexpr.org/