-
Notifications
You must be signed in to change notification settings - Fork 187
Rewrite codegen to use Vulkan Object & refactor API Dump extensively #2431
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
base: main
Are you sure you want to change the base?
Conversation
Simplifies codegen a bit by not needing to track the constants
Use the string instead of their numbers.
This makes transitioning to VulkanObject much easier, since extra spaces make the changes much longer for no good reason.
The include files only need the function declarations, so we can safely remove everything else from it. That does require moving the union and pNext function declarations into the .cpp files.
CI VulkanTools build queued with queue ID 468027. |
CI VulkanTools build # 4648 running. |
CI VulkanTools build # 4648 passed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-format wasn't working, so hard for me to really "check" code gen
(will re-review when that is fixed and comited)
tested the new layer and the output looks great and happy with it!
Major rewrite of the api_dump_generator.py script to use VulkanObject instead of its own OutputGenerator subclass. This rewrite attemps to change as little as possible in the generated code, to aid in validating the rewrite. But doing so introduced a few small errors as the default sorting was disabled. Subsequent commits will enable sorting and clean up the introduced mistakes.
Needed codegen changes to get the full count and new dump_format_array() templates to handle the types.
These were added to help keep straight the different kinds of output (value, pointer, array) and different output formats. But, with the consolidation work going on, they obscure actual differences in behavoir whenever working on the codegen.
The pNext chain iteration functions diverged between text, html, and json. This commit rectifies the situation, and correctly type-puns the incoming void* into a VkBaseInStructure using memcpy to get the sType and pNext correctly.
Makes sweeping changes to consolidate the printing of a type into a single function that handles all three output types. This makes it much easier to share common codegen logic between all output types, where previously it had to be duplicated.
Removes duplicate functions which only differed by whether they took a type by value or by reference. Greatly improves the readibility of api_dump.h since it removes a bunch of duplicated logic.
Modfies the existing helper functions in api_dump.h as well as adding new ones so that the code generator can directly call the appropriate types, instead of passing the dumping function to helper functions as a templated parameter. This makes the code easier to understand and walk through because the logic to print isn't being mixed with the logic to call other functions. This also simplifies the interface to helper functions by removing the function pointer argument, and any const/non-const overloads necessary for it.
Reduces the code variance between the generated output format code.
Allows codegen to not care at all about formatting. Also allows more use of '''<code>''' in codegen for long sequences of codegen, as before doing that would need to un-indent the block in order for the output to appear correct. Now, clang-format will remove the extra indents.
Hey Charles, is the GN build still happy? |
Uses a simple helper to remove redundant platform guards.
3704ebc
to
93e1e89
Compare
CI VulkanTools build queued with queue ID 469353. |
@mark-lunarg I do believe the GN build is good because no changes were made to the names of generated files, adding/removing files, and GN doesn't have any logic to run the generator anyhow means there isn't place for GN to be broken. Not to mention the android/chromium build steps which pass. |
CI VulkanTools build # 4649 running. |
CI VulkanTools build # 4649 failed. |
CI VulkanTools build queued with queue ID 469387. |
CI VulkanTools build # 4650 running. |
CI VulkanTools build # 4650 passed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some nits, but this looks great!
return 'union' if self.vk.structs[var.type].union else 'struct' | ||
return 'value' | ||
|
||
def write_value(self, output_format, var, parent): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a comment or update the name, what value
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You come up with a name that can mean either parameter or member, and is 'print me code that will dispatch the value needing to be printed without thinking of the word value'
Moves the handwritten implementations for various functions into a dedicated file. This makes reading the code easier and makes api_dump_generator.py only contain code that requires code generation to function. Also went through and cleaned up api_dump_generator.py a bit, using a consistent naming scheme, removing commented out code, and adding types to functions.
CI VulkanTools build queued with queue ID 469540. |
CI VulkanTools build # 4651 running. |
@mark-lunarg I lied, just pushed up a change that added a new .cpp file. But I did make sure to add that to the GN file because of your comment, so thanks for making it a thing to watch out for! |
CI VulkanTools build # 4651 passed. |
Rewrites Api Dump to use vulkan_object.py and fix many errors.
The PR starts by modifying the existing code to sort the generated code and cleanup small issues (like extra spaces). The rewrite itself took place over many commits, as the initial one was geared towards getting near-identical generated code output, then later on rewriting the codegen to be more maintainable and readable.
The biggest "issue" is that both base_generator.py and vulkan_object.py are checked into the repo with modifications necessary for API dump to run. In the future, these files will be fixed upstream in order to allow the local copies to be dropped. But since the changes needed aren't going to be up streamed, or just need more development time, it was decided to have the necessary changes to allow this rewrite to be reviewed and published while work continues upstream.