Skip to content
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

Publish GLSL_EXT_integer_dot_product #195

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

kpet
Copy link
Contributor

@kpet kpet commented Oct 4, 2022

Signed-off-by: Kevin Petit kevin.petit@arm.com
Change-Id: Ifc55b44217819cf5c6998eeb4aedcc8d7597e65d

@nihui
Copy link

nihui commented Oct 14, 2022

I wonder if there is currently good and handy type conversion function for packed4x8 type in integer_dot_product, like packHalf2x16() unpackHalf2x16() for packed fp16.
Shouldn't there be packInt4x8EXT() packUInt4x8EXT() unpackInt4x8EXT() unpackUInt4x8EXT() for packed 8bit integer ?

Existing conversions such as packSnorm4x8() will lose precision due to norm and are not suitable for use in computational tasks.

A common scenario:
Some gpu drivers do not support 8bit/16bit storage feature, so cannot read and write with 8bit integer
But we can pack 4x8bit into a 32bit and unpack it in shader for calculation, which can significantly reduce the bandwidth overhead of gpu memory

I admit that we could simulate this pack/unpack process with bit shift/or/and operations, but doing so is complex and limited and can be inefficient

@pdaniell-nv pdaniell-nv added the Vulkan Functionality applies to Vulkan API label Oct 25, 2022
@kpet kpet force-pushed the glsl-ext-insteger-dot-product branch from 263e5f3 to 5e827e0 Compare March 1, 2023 16:41
@gnl21
Copy link
Contributor

gnl21 commented Mar 3, 2023

I wonder if there is currently good and handy type conversion function for packed4x8 type in integer_dot_product

Currently no, I don't think there is. One could be added but ...

I admit that we could simulate this pack/unpack process with bit shift/or/and operations, but doing so is complex and limited and can be inefficient

I'm not sure why you say that they are limited, so perhaps you have something different in mind, but compilers should already understand that these are pack/unpack operations and be emitting good instruction sequences for them. I don't think we should expect any gain in efficiency from adding functions for this.

kpet added 3 commits June 7, 2023 17:25
Signed-off-by: Kevin Petit <kevin.petit@arm.com>
Change-Id: Ifc55b44217819cf5c6998eeb4aedcc8d7597e65d
Signed-off-by: Kevin Petit <kevin.petit@arm.com>
Change-Id: I67df435e5da65af460bacc40d5cba2379859516e
Signed-off-by: Kevin Petit <kevin.petit@arm.com>
@kpet kpet force-pushed the glsl-ext-insteger-dot-product branch 2 times, most recently from 5e827e0 to 8f73267 Compare June 7, 2023 18:08
@gnl21
Copy link
Contributor

gnl21 commented Jun 8, 2023

Thanks for the fixes. What's the state of implementations on this? Is it ready to be merged now?

@kpet
Copy link
Contributor Author

kpet commented Jun 9, 2023

Thanks for the fixes. What's the state of implementations on this? Is it ready to be merged now?

Thanks for the review! As far as I'm aware there are two prototypes from two different people, neither of which is complete. Neither author is able to continue work on their implementation. I expect this to be discussed in today's Vulkan ML teleconference and hopefully someone will be able to pick up the work. We could wait until we have a complete implementation to merge just in case implementation work turns up spec issues but I think this spec has been reviewed a good number of times by now so maybe publishing is helpful. I don't feel too strongly either way.

@oddhack
Copy link
Contributor

oddhack commented Aug 24, 2024

When resolving conflicts, please note that you will also need to move any changes in this branch to README.md, to corresponding asciidoc markup in README.adoc, and remove README.md .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Vulkan Functionality applies to Vulkan API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants