From: Henri Verbeet 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: Date: Fri, 5 Jun 2020 16:17:12 +0430 In-Reply-To: <4095fe3e-4428-d8ec-fae0-2d313164bd48@codeweavers.com> References: <20200604014915.986832-1-zfigura@codeweavers.com> <20200604014915.986832-4-zfigura@codeweavers.com> <4095fe3e-4428-d8ec-fae0-2d313164bd48@codeweavers.com> On Fri, 5 Jun 2020 at 02:55, Zebediah Figura wrote: > >> Is the idea then to introduce a new symbol table in struct > >> vkd3d_shader_parser? It seems a bit unfortunate that the table would be > >> duplicated, but after all I guess it's not a lot of duplication... > >> > > No, the idea would be that in spirv.c you'd use the symbol table to > > lookup the range information for the ID, and then use that information > > to lookup the counter/read information. E.g.: > > > > dcl_uav_typed u0[1:3], ..., space=4 > > ... > > store_uav_typed u0[1], ... > > > > The store_uav_typed handler would lookup u0 in the symbol table and > > find that it corresponds to [1:3] in register space 4. Index 1 in that > > range would then be descriptor 2 in that register space 4. It would > > then lookup the information for descriptor 2 in register space 4 in > > the vkd3d_shader_scan_info to check whether that's a readonly UAV. > > (There are of course variants on that, like doing the lookup when > > inserting u0 in the symbol table, etc. To some extent it also depends > > on what we do with the issue below.) > > I'm probably misunderstanding something, but how would we find the > register space and index in vkd3d_shader_scan_record_uav_read()? > It would need some kind of temporary lookup structure in vkd3d_shader_scan_dxbc(), but I don't think that needs to be a duplicate of the symbol table. > >>> 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.