Add support for the execution mode RegisterMapInterfaceINTEL#317
Add support for the execution mode RegisterMapInterfaceINTEL#317johnkslang merged 7 commits intoKhronosGroup:mainfrom
Conversation
Add Co-authored-by: Dmitry Sidorov <dmitry.sidorov@intel.com>
| { | ||
| "enumerant" : "FPGAKernelAttributesv2INTEL", | ||
| "value" : 6161, | ||
| "capabilities" : [ "FPGAKernelAttributesINTEL" ], | ||
| "extensions" : [ "SPV_INTEL_kernel_attributes" ], | ||
| "version" : "None" | ||
| }, |
There was a problem hiding this comment.
Did the headers generate correctly after this change? I see the new RegisterMapInterfaceINTEL enum, but not this new FPGAKernelAttributesv2INTEL capability.
There was a problem hiding this comment.
How and where do I check that? Do I have to run something locally? @MrSidims do you know how I can check the generated headers? I am going to try the steps shown in the repo README file.
There was a problem hiding this comment.
Could be because I did not add FPGAKernelAttributesv2INTEL to include/spirv/unified1/spirv.hpp11. Adding now.
There was a problem hiding this comment.
Thanks for catching this, I realize now that FPGAKernelAttributesv2INTEL should have been added everywhere just like FPGAKernelAttributesINTEL. Updated all files.
There was a problem hiding this comment.
Hmm, did you regenerate the headers after making this change? I'm seeing an error when I regenerate headers:
Error: "Capability" enumerant "FPGAMemoryAccessesINTEL" is out of order. It has value 5898 but follows the enumerant with value 6161
Instructions to regenerate the headers can be found here.
There was a problem hiding this comment.
No I did not. Looking into it now.
There was a problem hiding this comment.
Looks like there is a positioning requirement. Updated, and now the header generation command does not error out.
There was a problem hiding this comment.
Thanks, I don't see the out-of-order error anymore, but it looks like the FPGAKernelAttributesv2INTEL capability is now in the wrong place - see below.
There was a problem hiding this comment.
Updated in all files.
| { | ||
| "enumerant" : "FPGAKernelAttributesv2INTEL", | ||
| "value" : 6161, | ||
| "capabilities" : [ "FPGAKernelAttributesINTEL" ], | ||
| "extensions" : [ "SPV_INTEL_kernel_attributes" ], | ||
| "version" : "None" | ||
| }, |
There was a problem hiding this comment.
This enumerant is in the wrong place now - it is in the "Execution Mode" section and it needs to be in the "Capability" section. Be sure to maintain numerical order! Then regenerate headers after moving it.
There was a problem hiding this comment.
Thanks for catching this, moved FPGAKernelAttributesv2INTEL to capabilities section in all files, in the correct numerical order. Generated headers again and did not get any errors.
bashbaug
left a comment
bashbaug
left a comment
There was a problem hiding this comment.
Thanks, all LGTM now! Needs a non-Intel reviewer to merge.
|
@dneto0 / @johnkslang / @alan-baker Kindly review this PR. Thanks! :) |
Reminder ping :) @dneto0 / @johnkslang / @alan-baker thank you! |
4 participants
(Previous PR #309 was closed automatically by git when I synced my fork with Khronos repo - since I synced with the git hub UI and that required discarding my commit)
This SPIRV extension update will be introducing the new execution mode RegisterMapInterfaceINTEL. This PR adds support for this new mode.
NOTE
This execution mode is being added to an existing extension
SPV_INTEL_kernel_attributesand is guarded by a new capabilityFPGAKernelAttributesv2INTELwhich implicitly defines the existing capabilityFPGAKernelAttributesINTEL.