From: Zebediah Figura Subject: Re: HLSL offsetting Message-Id: Date: Sat, 25 Jun 2022 12:29:09 -0500 In-Reply-To: <07dca95c-e258-84bc-be37-dd3236a56ea0@codeweavers.com> References: <6a65d783-32b7-1869-622f-2898f70af302@codeweavers.com> <7cb1f3d1-0d20-1b6a-6328-132d0cd16dcf@codeweavers.com> <62467a7d-3a59-59b8-ef11-ec7b681f7e5d@codeweavers.com> <32508591-b482-cfb2-ef9d-afa0af351552@codeweavers.com> <145424ee-00e6-c6fa-ba80-83a8faadaa1a@codeweavers.com> <0b9be172-5c4a-b846-30b7-7be58d2ec767@codeweavers.com> <54839a65-9285-647f-836d-4eb074c7995f@codeweavers.com> <7bd6d5b0-0114-56d1-ea30-94ebdd830f8a@codeweavers.com> <1baa6160-f472-e80e-1033-b41c00a4fe73@codeweavers.com> <9a60c866-77fb-73cd-0e5d-26fbcff862a6@codeweavers.com> <58124ceb-c80e-14f8-094d-65edf961e6ae@codeweavers.com> <07dca95c-e258-84bc-be37-dd3236a56ea0@codeweavers.com> On 6/24/22 23:46, Francisco Casas wrote: > Hello, > > I attach the third version of the previous patch, applying the > suggestions made by Zeb, and some other changes that arose from them. > > The patch is based on the master branch, but I guess I will have to > update it if the recent patches from Giovanni are accepted, before > sending it for review. > Is it not possible to split up this patch, as I had asked earlier? There's also a couple things I notice from skimming: * I think add_load() should also be split into deref / component versions. Most of its users only need to count components. * Actually, should we be passing a deref instead of just a var to hlsl_new_component_load()? See e.g. add_cast(). That would make it a simple wrapper around hlsl_new_load_from_deref() [and the names would also need to be changed...] I'm not sure why I thought it made sense like this, frankly. * Do we need to always split up matrices into scalars? Should we make non-contiguous loads a problem for parse time? As long as we're trying to keep backend-specific details out of the HLSL IR, that seems potentially reasonable; it'd just require emitting multiple copy instructions for some cases. That would allow us to simplify split_copy() and add_matrix_scalar_load(), and I believe would thus also get rid of the need to pass multiple offsets to hlsl_new_load_from_deref().