From: Zebediah Figura Subject: Re: [PATCH vkd3d 4/5] vkd3d-shader: Return the register space of each used UAV and UAV counter in vkd3d_shader_scan_info. Message-Id: <5237c614-bc52-ca26-7971-b4487a213b06@codeweavers.com> Date: Fri, 5 Jun 2020 10:34:09 -0500 In-Reply-To: References: <20200604014915.986832-1-zfigura@codeweavers.com> <20200604014915.986832-4-zfigura@codeweavers.com> <4095fe3e-4428-d8ec-fae0-2d313164bd48@codeweavers.com> On 6/5/20 6:47 AM, Henri Verbeet wrote: >>>>> Perhaps more important is the question how this would work with ranges >>>>> larger than a single descriptor. I.e., how would for example a >>>>> "dcl_uav_raw u0[1:3], ..." declaration be represented in the >>>>> vkd3d_shader_scan_info structure? >>>>> >>>> >>>> It's hard for me to answer that without a full perspective as to how the >>>> indices are or might be used, but my best guess is that, if descriptor >>>> ranges are uploaded all at once, we'd want to track a single >>>> used/counter flag for the whole range. >>> >>> It probably needs tests to determine whether it's allowed in practice, >>> but the bytecode seems to allow for overlapping ranges. E.g.: >>> >>> dcl_uav_raw u0[1:4], ..., space=0 >>> dcl_uav_raw u1[3:6], ..., space=0 >>> >>> would make both u0[2] and u1[0] refer to descriptor 3 in register >>> space 0. >> >> In which case I guess we'd mark both ranges as read if descriptor 3 is >> read from. >> >>> But even without aliasing, it's not clear to me that e.g. >>> using the counter for u0[0] necessarily implies that all of u0 needs >>> one. >>> >> >> Yes, certainly, it's more conservative than is probably desirable, >> especially as I now examine spirv.c a little closer. Of course, I also >> don't know how much of that is redundancy and how much is a performance >> concern. >> >> While using e.g. a bitmask may be reasonable, I'm not sure how to >> reconcile that with unbounded or even particularly large descriptor >> arrays. Perhaps cutting off the bitmask size at a point makes sense (and >> treating anything further as "always used"?), but I don't have the >> experience to guess what a reasonable cutoff point is. > > My main concern would be that if we require an unnecessary UAV counter > descriptor that the application doesn't actually have, we'd fail to > compile the shader. Perhaps that's impossible, but we'd want to be > sure of that before designing the API in a way that makes resolving > that issue impossible. > If we extend the above proposal, perhaps something like: #define VKD3D_ARRAY_COUNT_UNBOUNDED (~0) struct vkd3d_uav_scan_info { unsigned int register_space, register_index, array_count; bool counter, read; } array_count doesn't necessarily have to reflect the actual declared array count (unless there's a reason to do that I'm not thinking of?) Thus if the array isn't indexed dynamically, we can fill one vkd3d_uav_scan_info entry for each access, each with an array_count of 1. If it is, we add one entry with the declared array count or VKD3D_ARRAY_COUNT_UNBOUNDED (if it's indexed dynamically). Does that work?