[PATCH v2 1/2] winevulkan: Manually fixup struct alignment for VkPipelineCreationFeedback.

[Zebediah Figura]

On 2/9/22 11:20, Georg Lehmann wrote:
> 
> 
> On 09.02.22 18:07, Zebediah Figura wrote:
>> On 2/9/22 10:39, Georg Lehmann wrote:
>>> We can't reasonably auto generate this because it's output in an
>>> otherwise input pNext chain.
>>
>> Aren't we going to need to do that anyway, though, for many other
>> functions, for wow64 support?
>>
>> It seems like it'd be more worthwhile to add infrastructure for
>> automatically generating this, or at least some of it.
>>
> 
> As far as I can tell this is the only pNext chain like this. We don't
> handle pure output chains, that's something that needs to be automated
> for wow64. But this issue is an unique edge case.

I'll admit this doesn't make me feel comfortable, not when Vulkan is a 
fast developing API that we are making an effort to keep up with. I.e. 
even if there aren't any others now it seems not unlikely we'll come 
across more of these in the future. And if we can find a solution that 
allows handling partial and wholly output chains in one breath, that 
seems nice...

It seems like most such structs are marked "returnedonly"; can we use 
that to determine what needs output conversion? It's missing from 
VkPipelineCreationFeedbackCreateInfo, but maybe that's an error.

> 
>>> +static void fixup_pipeline_feedback(VkPipelineCreationFeedback
>>> *feedback, uint32_t count)
>>> +{
>>> +#if defined(USE_STRUCT_CONVERSION)
>>> +    struct host_pipeline_feedback
>>> +    {
>>> +        VkPipelineCreationFeedbackFlags flags;
>>> +        uint64_t duration;
>>> +    } *host_feedback;
>>> +    int64_t i;
>>> +
>>> +    host_feedback = (void *) feedback;
>>> +
>>> +    for (i = count - 1; i >= 0; i--)
>>> +    {
>>> +        memmove(&feedback[i].duration, &host_feedback[i].duration,
>>> sizeof(uint64_t));
>>
>> Do we need memmove() here?
>>
> 
> These partially overlap so I think memmove is only way to do it safely.

Right, of course.

> 
>>> +        feedback[i].flags = host_feedback[i].flags;
>>> +    }
>>> +#else
>>> +    (void)feedback;
>>> +    (void)count;
>>
>> This is unnecessary; we don't use -Wunused-parameter.
>>
> 
> Personally I like being explicit here, but if you want I can send a new
> version with these removed.

We don't do this elsewhere in Wine, for whatever that's worth. (Nor do I 
think it's worthwhile, given how many unused parameters there are to 
Windows functions.) But regardless winevulkan isn't code I work on, so I 
don't really intend to prescribe coding style for it.