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.

Sign up for GitHub

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

Jump to bottom

[mlir][spirv] Support alias/restrict function argument decorations #76353

Merged
merged 11 commits into from Jan 6, 2024
Merged

[mlir][spirv] Support alias/restrict function argument decorations #76353

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ebd9634
[mlir][spirv] Support function argument decorations for ptr in the
sott0n Dec 22, 2023
2b64b3e
Support serializer and deserializer
sott0n Dec 27, 2023
d23a46d
addressed comments
sott0n Dec 29, 2023
f95fcce
addressed comments
sott0n Dec 31, 2023
6292d09
addressed comments
sott0n Jan 5, 2024
890afa1
move tests of function arguments
sott0n Jan 6, 2024
d75e852
remove test cases for random attribute from serialization test
sott0n Jan 6, 2024
0abc05f
addressed comments
sott0n Jan 6, 2024
6e382ee
apply clang-format
sott0n Jan 6, 2024
b08f51f
Merge remote-tracking branch 'origin/main' into spirv-func-arg
antiagainst Jan 6, 2024
e4e2764
Light touch on doc and error messages
antiagainst Jan 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
View file
Open in desktop
Expand Up @@ -976,19 +976,19 @@ LogicalResult spirv::FuncOp::verifyType() {
if (fnType.getNumResults() > 1)
return emitOpError("cannot have more than one result");

auto funcOp = dyn_cast<spirv::FuncOp>(getOperation());

auto hasDecorationAttr = [&](spirv::Decoration decoration,
unsigned argIndex) {
for (auto argAttr :
cast<FunctionOpInterface>(*funcOp).getArgAttrs(argIndex)) {
auto func = llvm::cast<FunctionOpInterface>(getOperation());
for (auto argAttr : cast<FunctionOpInterface>(func).getArgAttrs(argIndex)) {
if (argAttr.getName() != spirv::DecorationAttr::name)
continue;
if (auto decAttr = dyn_cast<spirv::DecorationAttr>(argAttr.getValue()))
antiagainst marked this conversation as resolved.
Show resolved Hide resolved
return decAttr.getValue() == decoration;
}
return false;
};

for (unsigned i = 0, e = funcOp.getNumArguments(); i != e; ++i) {
for (unsigned i = 0, e = this->getNumArguments(); i != e; ++i) {
Type param = fnType.getInputs()[i];
auto inputPtrType = dyn_cast<spirv::PointerType>(param);
if (!inputPtrType)
Expand Down
3 changes: 3 additions & 0 deletions mlir/lib/Target/SPIRV/Serialization/SerializeOps.cpp
View file
Open in desktop
Expand Up @@ -188,6 +188,9 @@ LogicalResult Serializer::processFuncParameter(spirv::FuncOp op) {
// Process decoration attributes of arguments.
auto funcOp = cast<FunctionOpInterface>(*op);
for (auto argAttr : funcOp.getArgAttrs(idx)) {
if (argAttr.getName() != DecorationAttr::name)
continue;

if (auto decAttr = dyn_cast<DecorationAttr>(argAttr.getValue())) {
antiagainst marked this conversation as resolved.
Show resolved Hide resolved
if (failed(processDecorationAttr(op->getLoc(), argValueID,
decAttr.getValue(), decAttr)))
Expand Down
36 changes: 25 additions & 11 deletions mlir/test/Dialect/SPIRV/IR/structure-ops.mlir
View file
Open in desktop
Expand Up @@ -319,43 +319,57 @@ spirv.module Logical GLSL450 {
// -----

// CHECK: spirv.func @arg_decoration_pointer(%{{.+}}: !spirv.ptr<i32, PhysicalStorageBuffer> {spirv.decoration = #spirv.decoration<Aliased>}) "None"
spirv.func @arg_decoration_pointer(%arg0: !spirv.ptr<i32, PhysicalStorageBuffer> { spirv.decoration = #spirv.decoration<Aliased>}) "None" {
spirv.Return
spirv.func @arg_decoration_pointer(%arg0: !spirv.ptr<i32, PhysicalStorageBuffer> { spirv.decoration = #spirv.decoration<Aliased> }) "None" {
spirv.Return
}

// -----

// CHECK: spirv.func @arg_decoration_pointer(%{{.+}}: !spirv.ptr<i32, PhysicalStorageBuffer> {spirv.decoration = #spirv.decoration<Restrict>}) "None"
spirv.func @arg_decoration_pointer(%arg0: !spirv.ptr<i32, PhysicalStorageBuffer> { spirv.decoration = #spirv.decoration<Restrict>}) "None" {
spirv.Return
spirv.func @arg_decoration_pointer(%arg0: !spirv.ptr<i32, PhysicalStorageBuffer> { spirv.decoration = #spirv.decoration<Restrict> }) "None" {
spirv.Return
}

// -----

// CHECK: spirv.func @arg_decoration_pointer(%{{.+}}: !spirv.ptr<!spirv.ptr<i32, PhysicalStorageBuffer>, Generic> {spirv.decoration = #spirv.decoration<AliasedPointer>}) "None"
spirv.func @arg_decoration_pointer(%arg0: !spirv.ptr<!spirv.ptr<i32, PhysicalStorageBuffer>, Generic> { spirv.decoration = #spirv.decoration<AliasedPointer>}) "None" {
spirv.Return
spirv.func @arg_decoration_pointer(%arg0: !spirv.ptr<!spirv.ptr<i32, PhysicalStorageBuffer>, Generic> { spirv.decoration = #spirv.decoration<AliasedPointer> }) "None" {
spirv.Return
}

// -----

// CHECK: spirv.func @arg_decoration_pointer(%{{.+}}: !spirv.ptr<!spirv.ptr<i32, PhysicalStorageBuffer>, Generic> {spirv.decoration = #spirv.decoration<RestrictPointer>}) "None"
spirv.func @arg_decoration_pointer(%arg0: !spirv.ptr<!spirv.ptr<i32, PhysicalStorageBuffer>, Generic> { spirv.decoration = #spirv.decoration<RestrictPointer>}) "None" {
spirv.Return
spirv.func @arg_decoration_pointer(%arg0: !spirv.ptr<!spirv.ptr<i32, PhysicalStorageBuffer>, Generic> { spirv.decoration = #spirv.decoration<RestrictPointer> }) "None" {
spirv.Return
}

// -----

// expected-error @+1 {{'spirv.func' op with physical buffer pointer must be decorated either 'Aliased' or 'Restrict'}}
spirv.func @no_arg_decoration_pointer(%arg0: !spirv.ptr<i32, PhysicalStorageBuffer>) "None" {
spirv.Return
spirv.Return
}

// -----

// expected-error @+1 {{'spirv.func' op with a pointer points to a physical buffer pointer must be decorated either 'AliasedPointer' or 'RestrictPointer'}}
spirv.func @no_arg_decoration_pointer(%arg0: !spirv.ptr<!spirv.ptr<i32, PhysicalStorageBuffer>, Input>) "None" {
spirv.Return
spirv.func @no_arg_decoration_pointer(%arg0: !spirv.ptr<!spirv.ptr<i32, PhysicalStorageBuffer>, Function>) "None" {
spirv.Return
}

// -----

// expected-error @+1 {{'spirv.func' op with physical buffer pointer must be decorated either 'Aliased' or 'Restrict'}}
spirv.func @no_decoration_name_attr(%arg0 : !spirv.ptr<i32, PhysicalStorageBuffer> { random_attr = #spirv.decoration<Aliased> }) "None" {
spirv.Return
}

// -----

// expected-error @+1 {{'spirv.func' op arguments may only have dialect attributes}}
spirv.func @no_decoration_name_attr(%arg0 : !spirv.ptr<i32, PhysicalStorageBuffer> { spirv.decoration = #spirv.decoration<Restrict>, random_attr = #spirv.decoration<Aliased> }) "None" {
spirv.Return
}

// -----
Expand Down
51 changes: 46 additions & 5 deletions mlir/test/Target/SPIRV/function-decorations.mlir
View file
Open in desktop
@@ -1,4 +1,4 @@
// RUN: mlir-translate -no-implicit-module -test-spirv-roundtrip -split-input-file %s | FileCheck %s
// RUN: mlir-translate -no-implicit-module -test-spirv-roundtrip -split-input-file -verify-diagnostics %s | FileCheck %s

spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader, Linkage], []> {
spirv.func @linkage_attr_test_kernel() "DontInline" attributes {} {
Expand All @@ -24,7 +24,7 @@ spirv.module PhysicalStorageBuffer64 GLSL450 requires #spirv.vce<v1.0,
[Shader, PhysicalStorageBufferAddresses], [SPV_KHR_physical_storage_buffer]> {
// CHECK-LABEL: spirv.func @func_arg_decoration_aliased(%{{.*}}: !spirv.ptr<i32, PhysicalStorageBuffer> {spirv.decoration = #spirv.decoration<Aliased>})
spirv.func @func_arg_decoration_aliased(
%arg0 : !spirv.ptr<i32, PhysicalStorageBuffer> { spirv.decoration = #spirv.decoration<Aliased>}
%arg0 : !spirv.ptr<i32, PhysicalStorageBuffer> { spirv.decoration = #spirv.decoration<Aliased> }
) "None" {
spirv.Return
}
Expand All @@ -36,7 +36,7 @@ spirv.module PhysicalStorageBuffer64 GLSL450 requires #spirv.vce<v1.0,
[Shader, PhysicalStorageBufferAddresses], [SPV_KHR_physical_storage_buffer]> {
// CHECK-LABEL: spirv.func @func_arg_decoration_restrict(%{{.*}}: !spirv.ptr<i32, PhysicalStorageBuffer> {spirv.decoration = #spirv.decoration<Restrict>})
spirv.func @func_arg_decoration_restrict(
%arg0 : !spirv.ptr<i32,PhysicalStorageBuffer> { spirv.decoration = #spirv.decoration<Restrict>}
%arg0 : !spirv.ptr<i32,PhysicalStorageBuffer> { spirv.decoration = #spirv.decoration<Restrict> }
) "None" {
spirv.Return
}
Expand All @@ -48,7 +48,7 @@ spirv.module PhysicalStorageBuffer64 GLSL450 requires #spirv.vce<v1.0,
[Shader, PhysicalStorageBufferAddresses], [SPV_KHR_physical_storage_buffer]> {
// CHECK-LABEL: spirv.func @func_arg_decoration_aliased_pointer(%{{.*}}: !spirv.ptr<!spirv.ptr<i32, PhysicalStorageBuffer>, Generic> {spirv.decoration = #spirv.decoration<AliasedPointer>})
spirv.func @func_arg_decoration_aliased_pointer(
%arg0 : !spirv.ptr<!spirv.ptr<i32,PhysicalStorageBuffer>, Generic> { spirv.decoration = #spirv.decoration<AliasedPointer>}
%arg0 : !spirv.ptr<!spirv.ptr<i32,PhysicalStorageBuffer>, Generic> { spirv.decoration = #spirv.decoration<AliasedPointer> }
) "None" {
spirv.Return
}
Expand All @@ -60,8 +60,49 @@ spirv.module PhysicalStorageBuffer64 GLSL450 requires #spirv.vce<v1.0,
[Shader, PhysicalStorageBufferAddresses], [SPV_KHR_physical_storage_buffer]> {
// CHECK-LABEL: spirv.func @func_arg_decoration_restrict_pointer(%{{.*}}: !spirv.ptr<!spirv.ptr<i32, PhysicalStorageBuffer>, Generic> {spirv.decoration = #spirv.decoration<RestrictPointer>})
spirv.func @func_arg_decoration_restrict_pointer(
%arg0 : !spirv.ptr<!spirv.ptr<i32,PhysicalStorageBuffer>, Generic> { spirv.decoration = #spirv.decoration<RestrictPointer>}
%arg0 : !spirv.ptr<!spirv.ptr<i32,PhysicalStorageBuffer>, Generic> { spirv.decoration = #spirv.decoration<RestrictPointer> }
) "None" {
spirv.Return
}
}

// -----

spirv.module PhysicalStorageBuffer64 GLSL450 requires #spirv.vce<v1.0,
[Shader, PhysicalStorageBufferAddresses], [SPV_KHR_physical_storage_buffer]> {
// CHECK-LABEL: spirv.func @fn1(%{{.*}}: i32, %{{.*}}: !spirv.ptr<i32, PhysicalStorageBuffer> {spirv.decoration = #spirv.decoration<Aliased>})
spirv.func @fn1(
%arg0: i32,
%arg1: !spirv.ptr<i32, PhysicalStorageBuffer> { spirv.decoration = #spirv.decoration<Aliased> }
) "None" {
spirv.Return
}

// CHECK-LABEL: spirv.func @fn2(%{{.*}}: !spirv.ptr<i32, PhysicalStorageBuffer> {spirv.decoration = #spirv.decoration<Aliased>}, %{{.*}}: !spirv.ptr<i32, PhysicalStorageBuffer> {spirv.decoration = #spirv.decoration<Restrict>})
spirv.func @fn2(
%arg0: !spirv.ptr<i32, PhysicalStorageBuffer> { spirv.decoration = #spirv.decoration<Aliased> },
%arg1: !spirv.ptr<i32, PhysicalStorageBuffer> { spirv.decoration = #spirv.decoration<Restrict>}
) "None" {
spirv.Return
}
}

// -----

spirv.module PhysicalStorageBuffer64 GLSL450 requires #spirv.vce<v1.0,
[Shader, PhysicalStorageBufferAddresses], [SPV_KHR_physical_storage_buffer]> {
// expected-error @+1 {{'spirv.func' op with physical buffer pointer must be decorated either 'Aliased' or 'Restrict'}}
spirv.func @no_decoration_name_attr(%arg0 : !spirv.ptr<i32, PhysicalStorageBuffer> { random_attr = #spirv.decoration<Aliased> }) "None" {
spirv.Return
}
}

// -----

spirv.module PhysicalStorageBuffer64 GLSL450 requires #spirv.vce<v1.0,
[Shader, PhysicalStorageBufferAddresses], [SPV_KHR_physical_storage_buffer]> {
// expected-error @+1 {{'spirv.func' op arguments may only have dialect attributes}}
antiagainst marked this conversation as resolved.
Show resolved Hide resolved
spirv.func @no_decoration_name_attr(%arg0 : !spirv.ptr<i32, PhysicalStorageBuffer> { spirv.decoration = #spirv.decoration<Restrict>, random_attr = #spirv.decoration<Aliased> }) "None" {
spirv.Return
}
}