[Mesa-dev] [PATCH 2/2] radv: ensure export arguments are always float

[Samuel Pitoiset]

On 12/6/18 3:18 PM, Rhys Perry wrote:
> ./deqp-vk --deqp-case=dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_32_to_16.scalar_uint0_frag
> should crash with something like:
> deqp-vk: lib/IR/Instructions.cpp:2590: static llvm::CastInst*
> llvm::CastInst::Create(llvm::Instruction::CastOps, llvm::Value*,
> llvm::Type*, const llvm::Twine&, llvm::Instruction*): Assertion
> `castIsValid(op, S, Ty) && "Invalid cast!"' failed.
> because it's trying to zext/sext a half float to a i32.
> 
> and ./deqp-vk --deqp-case=dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_32_to_16.scalar_uint0_vert
> should crash with something like:
> deqp-vk: lib/IR/Instructions.cpp:348: void
> llvm::CallInst::init(llvm::FunctionType*, llvm::Value*,
> llvm::ArrayRef<llvm::Value*>,
> llvm::ArrayRef<llvm::OperandBundleDefT<llvm::Value*> >, const
> llvm::Twine&): Assertion `(i >= FTy->getNumParams() ||
> FTy->getParamType(i) == Args[i]->getType()) && "Calling a function
> with a bad signature!"' failed.
> because it's calling the export intrinsic with incorrect argument types.
> 
> For both tests, it seems to only assert with LLVM 8 for some reason.

I guess you use a debug llvm build? Can you figure out what change 
introduces this crash?

> On Thu, 6 Dec 2018 at 13:31, Samuel Pitoiset <samuel.pitoiset at gmail.com> wrote:
>>
>>
>>
>> On 12/6/18 2:15 PM, Rhys Perry wrote:
>>> So that the signature is correct and consistent, the inputs to a export
>>> intrinsic should always be 32-bit floats.
>>>
>>> This and the previous commit fixes a large amount crashes from
>>> dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.input_output_int_*
>>> tests
>>>
>>
>> They don't crash for me? Please explain how to reproduce.
>>
>>> Fixes: b722b29f10d ('radv: add support for 16bit input/output')
>>> Signed-off-by: Rhys Perry <pendingchaos02 at gmail.com>
>>> ---
>>>    src/amd/vulkan/radv_nir_to_llvm.c | 6 +-----
>>>    1 file changed, 1 insertion(+), 5 deletions(-)
>>>
>>> diff --git a/src/amd/vulkan/radv_nir_to_llvm.c b/src/amd/vulkan/radv_nir_to_llvm.c
>>> index 0c91118e5a..90bcc8dbfe 100644
>>> --- a/src/amd/vulkan/radv_nir_to_llvm.c
>>> +++ b/src/amd/vulkan/radv_nir_to_llvm.c
>>> @@ -2464,12 +2464,8 @@ si_llvm_init_export_args(struct radv_shader_context *ctx,
>>>        } else
>>>                memcpy(&args->out[0], values, sizeof(values[0]) * 4);
>>>
>>> -     for (unsigned i = 0; i < 4; ++i) {
>>> -             if (!(args->enabled_channels & (1 << i)))
>>> -                     continue;
>>> -
>>> +     for (unsigned i = 0; i < 4; ++i)
>>>                args->out[i] = ac_to_float(&ctx->ac, args->out[i]);
>>> -     }
>>>    }
>>>
>>>    static void
>>>