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

[glslang] Add impl for several matrix and constant op. #3082

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

Conversation

ShchchowAMD
Copy link
Contributor

@ShchchowAMD ShchchowAMD commented Dec 7, 2022

Add impl for some ops, mainly for matrix and float / conv ops.

Finished op List:
(all of them have already been added to symbol table and related to a built-in op/function before, but not actually be implemented yet.)

EOpMatrixInverse
EOpTranspose
EOpPackSnorm2x16
EOpPackUnorm2x16
EOpPackHalf2x16
EOpUnpackSnorm2x16
EOpUnpackUnorm2x16
EOpUnpackHalf2x16
EOpDeterminant
EOpSinh
EOpCosh
EOpTanh
EOpAsinh
EOpAcosh
EOpAtanh
EOpFloatBitsToInt
EOpFloatBitsToUint
EOpIntBitsToFloat
EOpUintBitsToFloat
EOpDoubleBitsToInt64
EOpDoubleBitsToUint64
EOpInt64BitsToDouble
EOpUint64BitsToDouble
EOpMul (matrixCompMult)
EOpComma

@ShchchowAMD ShchchowAMD marked this pull request as ready for review December 12, 2022 05:31
@greg-lunarg greg-lunarg added the kokoro:run Trigger Google bot runs label Dec 15, 2022
@kokoro-team kokoro-team removed kokoro:run Trigger Google bot runs labels Dec 15, 2022
@@ -37,6 +37,7 @@
//

#include "localintermediate.h"
#include "../../SPIRV/hex_float.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a little ugly. The MachineIndependent module should be independent of any target module such as SPIRV.

There really is nothing about hex_float.h (and bitutils.h) that is related to SPIRV. They were brought in to support SPIRV and that is why they were placed there. They do have a spvutils namespace, but that is it.

How much work would it be to move these files into MachineIndependent and remove any reference of SPIRV from them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right.

The reason to include hex_float here is to keep those float/bit ops following same rules/implementation.

I'd have a try to refine the folder structure and namespace locally first.

@arcady-lunarg
Copy link
Contributor

@ShchchowAMD do you intend to get back to this at some point?

@ShchchowAMD
Copy link
Contributor Author

Thanks for reminding. I'll make a commit recently to fix issue discussed above.

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

Successfully merging this pull request may close these issues.

4 participants