Implement SPV_INTEL_global_variable_decorations extension #1389

vmaksimo · 2022-01-27T16:57:59Z

This change adds support of decorations applied to global variables through
spirv.Decorations metadata in LLVM IR.

These decorations are mostly intended to help code generation for FPGA devices:

HostAccessINTEL
InitModeINTEL
ImplementInCSRINTEL

Spec:
https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/DeviceGlobal/SPV_INTEL_global_variable_decorations.asciidoc

This change adds support of decorations applied to global variables through `spirv.Decorations` metadata in LLVM IR. These decorations are mostly intended to help code generation for FPGA devices: * HostAccessINTEL * InitModeINTEL * ImplementInCSRINTEL Spec: https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/DeviceGlobal/SPV_INTEL_global_variable_decorations.asciidoc

vmaksimo · 2022-01-28T07:30:03Z

Tagging @AlexeySotkin @MrSidims @KornevNikita @pauzinl to take a look

KornevNikita

Looks good for me

MrSidims · 2022-01-28T07:38:58Z

lib/SPIRV/SPIRVWriter.cpp

+      assert(NumOperands == 3 &&
+             "HostAccessINTEL requires exactly 3 extra operand");


According to spec, there are 2 operands.

Yeah, here's a bit incorrect naming/numbering of operands - it actually means that we expect metadata to contain 3 operands like !{i32 6147, i32 0, !{"Name"}}, where the decoration itself contains exactly 2 operands.
It was done alike to DecorationLinkageAttributes handling (see above).
I suppose the warning in the assert should be changed to avoid confusion.

MrSidims · 2022-01-28T07:54:23Z

test/transcoding/SPV_INTEL_global_variable_decorations/global_var_decorations.ll

+; CHECK-LLVM: @int_var = addrspace(1) global i32 42
+; CHECK-LLVM: @float_var = addrspace(1) global float 1.000000e+00
+; CHECK-LLVM: @bool_var = addrspace(1) global i1 false


Hm, that's interesting result. What would be 'OpenCL' IR representation of this feature? May be this comment should be applied sooner vs later.

Now it would be the same as for default behavior (IR as in CHECK-LLVM).

What is expected for the output of SPIRVToOCL pass? Should we just drop the decorations or to transform them to global variable attributes like attributes #0 = {"init_mode"="0" "host_access"="1"}? Or maybe something else?

IMHO, I'd expect we do drop the metadata in OpenCL IR mode until we have OpenCL extension for this feature. When/if we will have the OpenCL extension, we will have it's representation in OpenCL C and clang will translate it into something in LLVM IR. And that LLVM IR representation, I think, should be generated as output of SPIRVToOCL pass.

It looks like we have one. However it doesn't define OpenCL C representation. Probably it is not final draft.

lib/SPIRV/SPIRVWriter.cpp

lib/SPIRV/libSPIRV/SPIRVDecorate.cpp

gmlueck · 2022-01-28T12:57:47Z

FYI to @steffenlarsen since you were working in this area of the SPIRV-LLVM translator also.

MrSidims

LGTM

…up#1389) * Implement SPV_INTEL_global_variable_decorations extension This change adds support of decorations applied to global variables through `spirv.Decorations` metadata in LLVM IR. These decorations are mostly intended to help code generation for FPGA devices: * HostAccessINTEL * InitModeINTEL * ImplementInCSRINTEL Spec: https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/DeviceGlobal/SPV_INTEL_global_variable_decorations.asciidoc

…ons extension .. '"master"' -> '"xmain-web"' (1 commits) CONFLICT (content): Merge conflict in lib/SPIRV/libSPIRV/spirv_internal.hpp CONFLICT (content): Merge conflict in lib/SPIRV/libSPIRV/SPIRVNameMapEnum.h CONFLICT (content): Merge conflict in lib/SPIRV/libSPIRV/SPIRVDecorate.cpp CONFLICT (content): Merge conflict in include/LLVMSPIRVExtensions.inc commit 8937ecf Author: Viktoria Maximova <[email protected]> Date: Thu Feb 3 17:53:08 2022 +0300 Implement SPV_INTEL_global_variable_decorations extension (KhronosGroup#1389) * Implement SPV_INTEL_global_variable_decorations extension This change adds support of decorations applied to global variables through `spirv.Decorations` metadata in LLVM IR. These decorations are mostly intended to help code generation for FPGA devices: * HostAccessINTEL * InitModeINTEL * ImplementInCSRINTEL Spec: https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/DeviceGlobal/SPV_INTEL_global_variable_decorations.asciidoc

…le_decorations extension .. '"master"' -> '"xmain-web"' (1 commits) (KhronosGroup#117) * Resolve of merge (conflict) 8937ecf Implement SPV_INTEL_global_variable_decorations extension .. '"master"' -> '"xmain-web"' (1 commits) CONFLICT (content): Merge conflict in lib/SPIRV/libSPIRV/spirv_internal.hpp CONFLICT (content): Merge conflict in lib/SPIRV/libSPIRV/SPIRVNameMapEnum.h CONFLICT (content): Merge conflict in lib/SPIRV/libSPIRV/SPIRVDecorate.cpp CONFLICT (content): Merge conflict in include/LLVMSPIRVExtensions.inc commit 8937ecf Author: Viktoria Maximova <[email protected]> Date: Thu Feb 3 17:53:08 2022 +0300 Implement SPV_INTEL_global_variable_decorations extension (KhronosGroup#1389) * Implement SPV_INTEL_global_variable_decorations extension This change adds support of decorations applied to global variables through `spirv.Decorations` metadata in LLVM IR. These decorations are mostly intended to help code generation for FPGA devices: * HostAccessINTEL * InitModeINTEL * ImplementInCSRINTEL Spec: https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/DeviceGlobal/SPV_INTEL_global_variable_decorations.asciidoc * Resolve conflicts Co-authored-by: iclsrc <[email protected]> Co-authored-by: Sergey Semenov <[email protected]>

…al_variable_decorations extension .. '"master"' -> '"xmain-web"' (1 commits) (KhronosGroup#117) * Resolve of merge (conflict) 8937ecf Implement SPV_INTEL_global_variable_decorations extension .. '"master"' -> '"xmain-web"' (1 commits) CONFLICT (content): Merge conflict in lib/SPIRV/libSPIRV/spirv_internal.hpp CONFLICT (content): Merge conflict in lib/SPIRV/libSPIRV/SPIRVNameMapEnum.h CONFLICT (content): Merge conflict in lib/SPIRV/libSPIRV/SPIRVDecorate.cpp CONFLICT (content): Merge conflict in include/LLVMSPIRVExtensions.inc commit 8937ecf Author: Viktoria Maximova <[email protected]> Date: Thu Feb 3 17:53:08 2022 +0300 Implement SPV_INTEL_global_variable_decorations extension (KhronosGroup#1389) * Implement SPV_INTEL_global_variable_decorations extension This change adds support of decorations applied to global variables through `spirv.Decorations` metadata in LLVM IR. These decorations are mostly intended to help code generation for FPGA devices: * HostAccessINTEL * InitModeINTEL * ImplementInCSRINTEL Spec: https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/DeviceGlobal/SPV_INTEL_global_variable_decorations.asciidoc * Resolve conflicts Co-authored-by: iclsrc <[email protected]> Co-authored-by: Sergey Semenov <[email protected]>

…nsion (KhronosGroup#1389) * Implement SPV_INTEL_global_variable_decorations extension This change adds support of decorations applied to global variables through `spirv.Decorations` metadata in LLVM IR. These decorations are mostly intended to help code generation for FPGA devices: * HostAccessINTEL * InitModeINTEL * ImplementInCSRINTEL Spec: https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/DeviceGlobal/SPV_INTEL_global_variable_decorations.asciidoc

…sion (KhronosGroup#1389) * Implement SPV_INTEL_global_variable_decorations extension This change adds support of decorations applied to global variables through `spirv.Decorations` metadata in LLVM IR. These decorations are mostly intended to help code generation for FPGA devices: * HostAccessINTEL * InitModeINTEL * ImplementInCSRINTEL Spec: https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/DeviceGlobal/SPV_INTEL_global_variable_decorations.asciidoc

…nsion (KhronosGroup#1389) * Implement SPV_INTEL_global_variable_decorations extension This change adds support of decorations applied to global variables through `spirv.Decorations` metadata in LLVM IR. These decorations are mostly intended to help code generation for FPGA devices: * HostAccessINTEL * InitModeINTEL * ImplementInCSRINTEL Spec: https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/DeviceGlobal/SPV_INTEL_global_variable_decorations.asciidoc

…sion (KhronosGroup#1389) * Implement SPV_INTEL_global_variable_decorations extension This change adds support of decorations applied to global variables through `spirv.Decorations` metadata in LLVM IR. These decorations are mostly intended to help code generation for FPGA devices: * HostAccessINTEL * InitModeINTEL * ImplementInCSRINTEL Spec: https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/DeviceGlobal/SPV_INTEL_global_variable_decorations.asciidoc

…nsion (#1389) * Implement SPV_INTEL_global_variable_decorations extension This change adds support of decorations applied to global variables through `spirv.Decorations` metadata in LLVM IR. These decorations are mostly intended to help code generation for FPGA devices: * HostAccessINTEL * InitModeINTEL * ImplementInCSRINTEL Spec: https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/DeviceGlobal/SPV_INTEL_global_variable_decorations.asciidoc

…sion (#1389) * Implement SPV_INTEL_global_variable_decorations extension This change adds support of decorations applied to global variables through `spirv.Decorations` metadata in LLVM IR. These decorations are mostly intended to help code generation for FPGA devices: * HostAccessINTEL * InitModeINTEL * ImplementInCSRINTEL Spec: https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/DeviceGlobal/SPV_INTEL_global_variable_decorations.asciidoc

…nsion (KhronosGroup#1389) * Implement SPV_INTEL_global_variable_decorations extension This change adds support of decorations applied to global variables through `spirv.Decorations` metadata in LLVM IR. These decorations are mostly intended to help code generation for FPGA devices: * HostAccessINTEL * InitModeINTEL * ImplementInCSRINTEL Spec: https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/DeviceGlobal/SPV_INTEL_global_variable_decorations.asciidoc

…nsion (#1389) * Implement SPV_INTEL_global_variable_decorations extension This change adds support of decorations applied to global variables through `spirv.Decorations` metadata in LLVM IR. These decorations are mostly intended to help code generation for FPGA devices: * HostAccessINTEL * InitModeINTEL * ImplementInCSRINTEL Spec: https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/DeviceGlobal/SPV_INTEL_global_variable_decorations.asciidoc

This change is basically an update of KhronosGroup#1389 for spec changes. Implementation of the feature was based on Intel extension which was not officially published to Khronos. Now it has been split, updated, and published to Khronos by KhronosGroup/SPIRV-Registry#205 Summary of the things that have changed: * Capability names and a new capability was added * Values for decorations have been updated to enums * Decoration names and IDs have been changed Specs: https://github.com/KhronosGroup/SPIRV-Registry/blob/main/extensions/INTEL/SPV_INTEL_global_variable_fpga_decorations.asciidoc https://github.com/KhronosGroup/SPIRV-Registry/blob/main/extensions/INTEL/SPV_INTEL_global_variable_host_access.asciidoc

This change is basically an update of #1389 for spec changes. Implementation of the feature was based on Intel extension which was not officially published to Khronos. Now it has been split, updated, and published to Khronos by KhronosGroup/SPIRV-Registry#205 Summary of the things that have changed: Capability names and a new capability was added Values for decorations have been updated to enums Decoration names and IDs have been changed Specs: https://github.com/KhronosGroup/SPIRV-Registry/blob/main/extensions/INTEL/SPV_INTEL_global_variable_fpga_decorations.asciidoc https://github.com/KhronosGroup/SPIRV-Registry/blob/main/extensions/INTEL/SPV_INTEL_global_variable_host_access.asciidoc

…p#2174) This change is basically an update of KhronosGroup#1389 for spec changes. Implementation of the feature was based on Intel extension which was not officially published to Khronos. Now it has been split, updated, and published to Khronos by KhronosGroup/SPIRV-Registry#205 Summary of the things that have changed: Capability names and a new capability was added Values for decorations have been updated to enums Decoration names and IDs have been changed Specs: https://github.com/KhronosGroup/SPIRV-Registry/blob/main/extensions/INTEL/SPV_INTEL_global_variable_fpga_decorations.asciidoc https://github.com/KhronosGroup/SPIRV-Registry/blob/main/extensions/INTEL/SPV_INTEL_global_variable_host_access.asciidoc

This change is basically an update of #1389 for spec changes. Implementation of the feature was based on Intel extension which was not officially published to Khronos. Now it has been split, updated, and published to Khronos by KhronosGroup/SPIRV-Registry#205 Summary of the things that have changed: Capability names and a new capability was added Values for decorations have been updated to enums Decoration names and IDs have been changed Specs: https://github.com/KhronosGroup/SPIRV-Registry/blob/main/extensions/INTEL/SPV_INTEL_global_variable_fpga_decorations.asciidoc https://github.com/KhronosGroup/SPIRV-Registry/blob/main/extensions/INTEL/SPV_INTEL_global_variable_host_access.asciidoc Co-authored-by: Viktoria Maximova <[email protected]>

KornevNikita approved these changes Jan 28, 2022

View reviewed changes

MrSidims reviewed Jan 28, 2022

View reviewed changes

lib/SPIRV/libSPIRV/SPIRVDecorate.cpp Show resolved Hide resolved

Apply suggestions and aling the implementation to the spec

aa05fcb

vmaksimo requested a review from MrSidims February 2, 2022 16:39

MrSidims approved these changes Feb 3, 2022

View reviewed changes

AlexeySotkin approved these changes Feb 3, 2022

View reviewed changes

AlexeySotkin merged commit 8937ecf into KhronosGroup:master Feb 3, 2022

vmaksimo mentioned this pull request Oct 6, 2023

Update decorations for global variables #2174

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement SPV_INTEL_global_variable_decorations extension #1389

Implement SPV_INTEL_global_variable_decorations extension #1389

vmaksimo commented Jan 27, 2022

vmaksimo commented Jan 28, 2022

KornevNikita left a comment

MrSidims Jan 28, 2022

vmaksimo Jan 28, 2022

MrSidims Jan 28, 2022

vmaksimo Jan 28, 2022

AlexeySotkin Feb 2, 2022

AlexeySotkin Feb 2, 2022

gmlueck commented Jan 28, 2022

MrSidims left a comment

		assert(NumOperands == 3 &&
		"HostAccessINTEL requires exactly 3 extra operand");

Implement SPV_INTEL_global_variable_decorations extension #1389

Implement SPV_INTEL_global_variable_decorations extension #1389

Conversation

vmaksimo commented Jan 27, 2022

vmaksimo commented Jan 28, 2022

KornevNikita left a comment

Choose a reason for hiding this comment

MrSidims Jan 28, 2022

Choose a reason for hiding this comment

vmaksimo Jan 28, 2022

Choose a reason for hiding this comment

MrSidims Jan 28, 2022

Choose a reason for hiding this comment

vmaksimo Jan 28, 2022

Choose a reason for hiding this comment

AlexeySotkin Feb 2, 2022

Choose a reason for hiding this comment

AlexeySotkin Feb 2, 2022

Choose a reason for hiding this comment

gmlueck commented Jan 28, 2022

MrSidims left a comment

Choose a reason for hiding this comment