-
Notifications
You must be signed in to change notification settings - Fork 402
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Vertex input binding allocation needs to take into account unused bindings #2234
Comments
I don't know, but I assume the reason for that is that the lower bindings are reserved for user buffers (e.g. |
That was the original idea, in the days before things got complicated with auxiliary buffers. I think the proper way to address this is for MoltenVK to move to using Metal argument buffers always (or at least by default). At the very least, that will free up more discrete buffers for use as auxiliary buffers, and later on we could evolve to having those auxiliary buffers in their own Metal argument buffer. Which is to say, I'd like to stay away from overly dramatic refactoring of discrete buffers until we've made that move. I am scheduled to start that move to better Metal argument buffers within by the end of this month. |
In short, MoltenVK does not account for unused vertex input binding locations which may lead to no free location for extra buffers MoltenVK requires like implicit buffers. We need to filter unused locations at pipeline creation, reassign locations to fill unused ones, and later allocate required locations by MoltenVK.
Longer explanation on #2222 and issues with current design:
As of now, MoltenVK reserves the vertex input binding locations requested by the application when creating the pipeline, and will allocate implicit buffers after. MoltenVK relocates bindings the following way: if user request binding location 0 for a buffer, it'll be bound to max binding location from Metal, location 1 to max_location - 1, 2 to max_location - 2, and so on. What happens when the user requests max_locations? Implicit buffer locations will be assigned max_location - max_location - 1, and since the value is unsigned, assigned location will be UINT_MAX. Later when we process buffer binding calls, we check that the Metal binding location is less than the implicit buffer location. Worst case scenario is where the implicit buffer location is UINT_MAX (user requested all binding locations) and no unsinged number is bigger than UINT_MAX, which leads to no buffers being bound. This is what #2222 partially fixed. What it does not solve is freeing unused bindings for other usages like implicit buffers.
Proposed changes:
Thoughts @billhollings
The text was updated successfully, but these errors were encountered: