[mlir][spirv] Fix shader ABI attribute prefix and add verification

[antiagainst]

Summary

This commit fixes shader ABI attributes to use spv. as the prefix
so that they match the dialect's namespace. This enables use to
add verification hooks in the SPIR-V dialect to verify them.

[merge_guards_bot]

Unit tests: pass. 61169 tests passed, 0 failed and 728 were skipped.

clang-tidy: fail. Please fix clang-tidy findings.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[antiagainst]

Add more tests

[merge_guards_bot]

Unit tests: pass. 61169 tests passed, 0 failed and 728 were skipped.

clang-tidy: fail. Please fix clang-tidy findings.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[mravishankar]

Thanks for fixing the naming!

mlir/lib/Dialect/SPIRV/SPIRVDialect.cpp

664

"verifyRegionArgResultAttribute" is misleading. What is intended by ArgResult? How about just "verifyRegionArgAttribute"

696

I am not sure it is well defined to have a InterfaceVarABIAttr for the result. So far the GPU to SPIR-V lowering assumes that entry point functions have no return values, and the LowerABIAttributesPass does not handle ABI attributes on the results. I tried to add that, but wasnt sure about the semantics there. So better to not handle it, and maybe error out when such attribute is specified on the result.

[kiszk]

mlir/lib/Dialect/SPIRV/SPIRVDialect.cpp

679

nit: attribute -> attributes?

[kiszk]

mlir/test/Dialect/SPIRV/target-and-abi.mlir

55

nit: attribute-> attributes?
Same comments to other places

[antiagainst]

Address comments

[merge_guards_bot]

Unit tests: pass. 61169 tests passed, 0 failed and 728 were skipped.

clang-tidy: fail. Please fix clang-tidy findings.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[antiagainst]

mlir/lib/Dialect/SPIRV/SPIRVDialect.cpp

664

Changed to verifyRegionAttribute and added comments to avoid the confusion.

696

What are the problems you see for attaching InterfaceVarABIAttr to a result? Isn't the semantics of it a write-only buffer?

In the verification code implemented by this CL, I'm intentionally not checking which op the attribute is attached to so that we can allow "carrying over" these attributes on some intermediate state in the lowering process. (For example somebody might want to create an intermediate function op of some sort maybe.) So the verification here only focuses on the attributes themselves; not really paying attention to the op or region arg/result index/position, etc. I get your points that we are not supporting InterfaceVarABIAttr in GPU lowering yet but this still seems a step forward that is nice to have: previously we don't verify these attributes at all, now we at least make sure they are correct internally. :)

[mravishankar]

mlir/lib/Dialect/SPIRV/SPIRVDialect.cpp

696

What would the function look like

func(%arg0 : memref<..>) -> memref<..> { ... }

This suggests that you are "returning" a memref, which makes ownership of that memref tricky to define. You could say that the lowering will convert this function into

func(%arg0 : memref<...>, %arg1 : memref<...>) {...}

and then say %arg1 is for the result. All this needs to be handled in the LowerABIAttributesPass. I was waiting for having a realistic use case for this. Currently when lowering from GPU dialect to SPIR-V, the entry function has void return type. So would rather have the verifier return error on this (cause its not implemented).

[antiagainst]

mlir/lib/Dialect/SPIRV/SPIRVDialect.cpp

696

Right, it depends on the definition of the op and its semantics. It might not make sense for func given that we have a relatively clear definition and understanding. I could have my funky op that uses results for some other purposes. Not advocating that we should intentionally support such cases; it's just that IMO that is a separate concern than verifying the attribute itself: it's more of resulting from the op's semantics than the spv.* attributes' semantics.

This is just another incarnation of the open-vs-closed trade-off we are facing. :) Typically I'd be very worried if being open means we can have many unexpected ways to break. Then I'd prefer to enumerate what we have at hand and support them explicitly and go from there to extend as needed. But for this one it seems it's not gonna bite us that much (and whatever unexpected places it gets attached to have some minimal verification at least). So I'd think a little openness does not hurt that much. :)

mlir/include/mlir/Dialect/SPIRV/SPIRVDialect.h

 //===- SPIRVDialect.h - MLIR SPIR-V dialect ---------------------*- C++ -*-===//
 //
 // Part of the MLIR Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file declares the SPIR-V dialect in MLIR.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef MLIR_DIALECT_SPIRV_SPIRVDIALECT_H_
 #define MLIR_DIALECT_SPIRV_SPIRVDIALECT_H_
 
 #include "mlir/IR/Dialect.h"
 
 namespace mlir {
 namespace spirv {
 
 enum class Decoration : uint32_t;
 
 class SPIRVDialect : public Dialect {
 public:
public:
   Type parseType(DialectAsmParser &parser) const override;
 
   /// Prints a type registered to this dialect.
   void printType(Type type, DialectAsmPrinter &os) const override;
 
   /// Provides a hook for materializing a constant to this dialect.
   Operation *materializeConstant(OpBuilder &builder, Attribute value, Type type,
                                  Location loc) override;
+
+  /// Provides a hook for verifying SPIR-V dialect attributes attached to the
+  /// given op.
+  LogicalResult verifyOperationAttribute(Operation *op,
+                                         NamedAttribute attribute) override;
+
+  /// Provides a hook for verifying SPIR-V dialect attributes attached to the
+  /// given op's region argument.
+  LogicalResult verifyRegionArgAttribute(Operation *op, unsigned regionIndex,
+                                         unsigned argIndex,
+                                         NamedAttribute attribute) override;
+
+  /// Provides a hook for verifying SPIR-V dialect attributes attached to the
+  /// given op's region result.
+  LogicalResult verifyRegionResultAttribute(Operation *op, unsigned regionIndex,
+                                            unsigned resultIndex,
+                                            NamedAttribute attribute) override;
 };
 
 } // end namespace spirv
 } // end namespace mlir
 
 #endif // MLIR_DIALECT_SPIRV_SPIRVDIALECT_H_

mlir/lib/Dialect/SPIRV/SPIRVDialect.cpp

 //===- LLVMDialect.cpp - MLIR SPIR-V dialect ------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines the SPIR-V dialect in MLIR.
 //
 //===----------------------------------------------------------------------===//
 
 #include "mlir/Dialect/SPIRV/SPIRVDialect.h"
 #include "mlir/Dialect/SPIRV/SPIRVOps.h"
 #include "mlir/Dialect/SPIRV/SPIRVTypes.h"
+#include "mlir/Dialect/SPIRV/TargetAndABI.h"
 #include "mlir/IR/Builders.h"
 #include "mlir/IR/DialectImplementation.h"
 #include "mlir/IR/MLIRContext.h"
 #include "mlir/IR/StandardTypes.h"
 #include "mlir/Parser.h"
 #include "mlir/Support/StringExtras.h"
 #include "mlir/Transforms/InliningUtils.h"
 #include "llvm/ADT/DenseMap.h"

 Operation *SPIRVDialect::materializeConstant(OpBuilder &builder,
                                              Attribute value, Type type,
                                              Location loc) {
   if (!ConstantOp::isBuildableWith(type))
     return nullptr;
 
   return builder.create<spirv::ConstantOp>(loc, type, value);
 }
+
+//===----------------------------------------------------------------------===//
+// Shader Interface ABI
+//===----------------------------------------------------------------------===//
+
+LogicalResult SPIRVDialect::verifyOperationAttribute(Operation *op,
+                                                     NamedAttribute attribute) {
+  StringRef symbol = attribute.first.strref();
+  Attribute attr = attribute.second;
+
+  if (symbol != spirv::getEntryPointABIAttrName())
+    return op->emitError("found unsupported '")
+           << symbol << "' attribute on operation";
+
+  if (!spirv::EntryPointABIAttr::classof(attr))
+    return op->emitError("'")
+           << symbol
+           << "' attribute must be a dictionary attribute containing one "
+              "integer elements attribute: 'local_size'";
+
+  return success();
+}
+
+static LogicalResult verifyRegionArgResultAttribute(Location loc,
+                                                    NamedAttribute attribute,
+                                                    bool forArg) {
+  StringRef symbol = attribute.first.strref();
+  Attribute attr = attribute.second;
+
+  if (symbol != spirv::getInterfaceVarABIAttrName())
+    return emitError(loc, "found unsupported '")
+           << symbol << "' attribute on " << (forArg ? "argument" : "result");
+
+  if (!spirv::InterfaceVarABIAttr::classof(attr))
+    return emitError(loc, "'")
+           << symbol
+           << "' attribute must be a dictionary attribute containing three "
+              "integer attribute: 'descriptor_set', 'binding', and "
+              "'storage_class'";
+
+  return success();
+}
+
+LogicalResult SPIRVDialect::verifyRegionArgAttribute(Operation *op,
+                                                     unsigned /*regionIndex*/,
+                                                     unsigned /*argIndex*/,
+                                                     NamedAttribute attribute) {
+  return verifyRegionArgResultAttribute(op->getLoc(), attribute,
+                                        /*forArg=*/true);
+}
+
+LogicalResult SPIRVDialect::verifyRegionResultAttribute(
+    Operation *op, unsigned /*regionIndex*/, unsigned /*resultIndex*/,
+    NamedAttribute attribute) {
+  return verifyRegionArgResultAttribute(op->getLoc(), attribute,
+                                        /*forArg=*/false);
+}

mlir/lib/Dialect/SPIRV/TargetAndABI.cpp

 //===- SPIRVLowering.cpp - Standard to SPIR-V dialect conversion--===//
 //
 // Part of the MLIR Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "mlir/Dialect/SPIRV/TargetAndABI.h"
 #include "mlir/Dialect/SPIRV/SPIRVTypes.h"
 #include "mlir/IR/Operation.h"
 
 using namespace mlir;
 
 namespace mlir {
 #include "mlir/Dialect/SPIRV/TargetAndABI.cpp.inc"
 }
 
 StringRef mlir::spirv::getInterfaceVarABIAttrName() {
-  return "spirv.interface_var_abi";
+  return "spv.interface_var_abi";
 }
 
 mlir::spirv::InterfaceVarABIAttr
 mlir::spirv::getInterfaceVarABIAttr(unsigned descriptorSet, unsigned binding,
                                     spirv::StorageClass storageClass,
                                     MLIRContext *context) {
   Type i32Type = IntegerType::get(32, context);
   return mlir::spirv::InterfaceVarABIAttr::get(
       IntegerAttr::get(i32Type, descriptorSet),
       IntegerAttr::get(i32Type, binding),
       IntegerAttr::get(i32Type, static_cast<int64_t>(storageClass)), context);
 }
 
 StringRef mlir::spirv::getEntryPointABIAttrName() {
-  return "spirv.entry_point_abi";
+  return "spv.entry_point_abi";
 }
 
 mlir::spirv::EntryPointABIAttr
 mlir::spirv::getEntryPointABIAttr(ArrayRef<int32_t> localSize,
                                   MLIRContext *context) {
   assert(localSize.size() == 3);
   return mlir::spirv::EntryPointABIAttr::get(
       DenseElementsAttr::get<int32_t>(
           VectorType::get(3, IntegerType::get(32, context)), localSize)
           .cast<DenseIntElementsAttr>(),
       context);
 }

mlir/test/Conversion/GPUToSPIRV/load-store.mlir

module attributes {gpu.container_module} {
 
   // CHECK-LABEL: spv.module "Logical" "GLSL450"
   module @kernels attributes {gpu.kernel_module} {
     // CHECK-DAG: spv.globalVariable [[WORKGROUPSIZEVAR:@.*]] built_in("WorkgroupSize") : !spv.ptr<vector<3xi32>, Input>
     // CHECK-DAG: spv.globalVariable [[NUMWORKGROUPSVAR:@.*]] built_in("NumWorkgroups") : !spv.ptr<vector<3xi32>, Input>
     // CHECK-DAG: spv.globalVariable [[LOCALINVOCATIONIDVAR:@.*]] built_in("LocalInvocationId") : !spv.ptr<vector<3xi32>, Input>
     // CHECK-DAG: spv.globalVariable [[WORKGROUPIDVAR:@.*]] built_in("WorkgroupId") : !spv.ptr<vector<3xi32>, Input>
     // CHECK-LABEL:    func @load_store_kernel
-    // CHECK-SAME: [[ARG0:%.*]]: !spv.ptr<!spv.struct<!spv.array<48 x f32 [4]> [0]>, StorageBuffer> {spirv.interface_var_abi = {binding = 0 : i32, descriptor_set = 0 : i32, storage_class = 12 : i32{{[}][}]}}
-    // CHECK-SAME: [[ARG1:%.*]]: !spv.ptr<!spv.struct<!spv.array<48 x f32 [4]> [0]>, StorageBuffer> {spirv.interface_var_abi = {binding = 1 : i32, descriptor_set = 0 : i32, storage_class = 12 : i32{{[}][}]}}
-    // CHECK-SAME: [[ARG2:%.*]]: !spv.ptr<!spv.struct<!spv.array<48 x f32 [4]> [0]>, StorageBuffer> {spirv.interface_var_abi = {binding = 2 : i32, descriptor_set = 0 : i32, storage_class = 12 : i32{{[}][}]}}
-    // CHECK-SAME: [[ARG3:%.*]]: i32 {spirv.interface_var_abi = {binding = 3 : i32, descriptor_set = 0 : i32, storage_class = 12 : i32{{[}][}]}}
-    // CHECK-SAME: [[ARG4:%.*]]: i32 {spirv.interface_var_abi = {binding = 4 : i32, descriptor_set = 0 : i32, storage_class = 12 : i32{{[}][}]}}
-    // CHECK-SAME: [[ARG5:%.*]]: i32 {spirv.interface_var_abi = {binding = 5 : i32, descriptor_set = 0 : i32, storage_class = 12 : i32{{[}][}]}}
-    // CHECK-SAME: [[ARG6:%.*]]: i32 {spirv.interface_var_abi = {binding = 6 : i32, descriptor_set = 0 : i32, storage_class = 12 : i32{{[}][}]}}
+    // CHECK-SAME: [[ARG0:%.*]]: !spv.ptr<!spv.struct<!spv.array<48 x f32 [4]> [0]>, StorageBuffer> {spv.interface_var_abi = {binding = 0 : i32, descriptor_set = 0 : i32, storage_class = 12 : i32{{[}][}]}}
+    // CHECK-SAME: [[ARG1:%.*]]: !spv.ptr<!spv.struct<!spv.array<48 x f32 [4]> [0]>, StorageBuffer> {spv.interface_var_abi = {binding = 1 : i32, descriptor_set = 0 : i32, storage_class = 12 : i32{{[}][}]}}
+    // CHECK-SAME: [[ARG2:%.*]]: !spv.ptr<!spv.struct<!spv.array<48 x f32 [4]> [0]>, StorageBuffer> {spv.interface_var_abi = {binding = 2 : i32, descriptor_set = 0 : i32, storage_class = 12 : i32{{[}][}]}}
+    // CHECK-SAME: [[ARG3:%.*]]: i32 {spv.interface_var_abi = {binding = 3 : i32, descriptor_set = 0 : i32, storage_class = 12 : i32{{[}][}]}}
+    // CHECK-SAME: [[ARG4:%.*]]: i32 {spv.interface_var_abi = {binding = 4 : i32, descriptor_set = 0 : i32, storage_class = 12 : i32{{[}][}]}}
+    // CHECK-SAME: [[ARG5:%.*]]: i32 {spv.interface_var_abi = {binding = 5 : i32, descriptor_set = 0 : i32, storage_class = 12 : i32{{[}][}]}}
+    // CHECK-SAME: [[ARG6:%.*]]: i32 {spv.interface_var_abi = {binding = 6 : i32, descriptor_set = 0 : i32, storage_class = 12 : i32{{[}][}]}}
     gpu.func @load_store_kernel(%arg0: memref<12x4xf32>, %arg1: memref<12x4xf32>, %arg2: memref<12x4xf32>, %arg3: index, %arg4: index, %arg5: index, %arg6: index)
       attributes  {gpu.kernel} {
       // CHECK: [[ADDRESSWORKGROUPID:%.*]] = spv._address_of [[WORKGROUPIDVAR]]
       // CHECK: [[WORKGROUPID:%.*]] = spv.Load "Input" [[ADDRESSWORKGROUPID]]
       // CHECK: [[WORKGROUPIDX:%.*]] = spv.CompositeExtract [[WORKGROUPID]]{{\[}}0 : i32{{\]}}
       // CHECK: [[ADDRESSLOCALINVOCATIONID:%.*]] = spv._address_of [[LOCALINVOCATIONIDVAR]]
       // CHECK: [[LOCALINVOCATIONID:%.*]] = spv.Load "Input" [[ADDRESSLOCALINVOCATIONID]]
       // CHECK: [[LOCALINVOCATIONIDX:%.*]] = spv.CompositeExtract [[LOCALINVOCATIONID]]{{\[}}0 : i32{{\]}}

mlir/test/Conversion/GPUToSPIRV/simple.mlir

 // RUN: mlir-opt -pass-pipeline='convert-gpu-to-spirv{workgroup-size=32,4}' %s -o - | FileCheck %s
 
 module attributes {gpu.container_module} {
 
   module @kernels attributes {gpu.kernel_module} {
     // CHECK:       spv.module "Logical" "GLSL450" {
     // CHECK-LABEL: func @kernel_1
-    // CHECK-SAME: {{%.*}}: f32 {spirv.interface_var_abi = {binding = 0 : i32, descriptor_set = 0 : i32, storage_class = 12 : i32{{[}][}]}}
-    // CHECK-SAME: {{%.*}}: !spv.ptr<!spv.struct<!spv.array<12 x f32 [4]> [0]>, StorageBuffer> {spirv.interface_var_abi = {binding = 1 : i32, descriptor_set = 0 : i32, storage_class = 12 : i32{{[}][}]}}
-    // CHECK-SAME: spirv.entry_point_abi = {local_size = dense<[32, 4, 1]> : vector<3xi32>}
+    // CHECK-SAME: {{%.*}}: f32 {spv.interface_var_abi = {binding = 0 : i32, descriptor_set = 0 : i32, storage_class = 12 : i32{{[}][}]}}
+    // CHECK-SAME: {{%.*}}: !spv.ptr<!spv.struct<!spv.array<12 x f32 [4]> [0]>, StorageBuffer> {spv.interface_var_abi = {binding = 1 : i32, descriptor_set = 0 : i32, storage_class = 12 : i32{{[}][}]}}
+    // CHECK-SAME: spv.entry_point_abi = {local_size = dense<[32, 4, 1]> : vector<3xi32>}
     gpu.func @kernel_1(%arg0 : f32, %arg1 : memref<12xf32>) attributes {gpu.kernel} {
       // CHECK: spv.Return
       gpu.return
     }
     // CHECK: attributes {capabilities = ["Shader"], extensions = ["SPV_KHR_storage_buffer_storage_class"]}
   }
 
   func @foo() {
     %0 = "op"() : () -> (f32)
     %1 = "op"() : () -> (memref<12xf32>)
     %cst = constant 1 : index
     "gpu.launch_func"(%cst, %cst, %cst, %cst, %cst, %cst, %0, %1) { kernel = "kernel_1", kernel_module = @kernels }
         : (index, index, index, index, index, index, f32, memref<12xf32>) -> ()
     return
   }
 }

mlir/test/Dialect/SPIRV/Transforms/abi-load-store.mlir

spv.module "Logical" "GLSL450" {
   // CHECK-DAG: spv.globalVariable [[VAR1:@.*]] bind(0, 1) : !spv.ptr<!spv.struct<!spv.array<12 x !spv.array<4 x f32 [4]> [16]> [0]>, StorageBuffer>
   // CHECK-DAG: spv.globalVariable [[VAR2:@.*]] bind(0, 2) : !spv.ptr<!spv.struct<!spv.array<12 x !spv.array<4 x f32 [4]> [16]> [0]>, StorageBuffer>
   // CHECK-DAG: spv.globalVariable [[VAR3:@.*]] bind(0, 3) : !spv.ptr<!spv.struct<i32 [0]>, StorageBuffer>
   // CHECK-DAG: spv.globalVariable [[VAR4:@.*]] bind(0, 4) : !spv.ptr<!spv.struct<i32 [0]>, StorageBuffer>
   // CHECK-DAG: spv.globalVariable [[VAR5:@.*]] bind(0, 5) : !spv.ptr<!spv.struct<i32 [0]>, StorageBuffer>
   // CHECK-DAG: spv.globalVariable [[VAR6:@.*]] bind(0, 6) : !spv.ptr<!spv.struct<i32 [0]>, StorageBuffer>
   // CHECK: func [[FN:@.*]]()
   func @load_store_kernel(%arg0: !spv.ptr<!spv.struct<!spv.array<12 x !spv.array<4 x f32>>>, StorageBuffer>
-                          {spirv.interface_var_abi = {binding = 0 : i32,
-                                                      descriptor_set = 0 : i32,
-                                                      storage_class = 12 : i32}},
+                          {spv.interface_var_abi = {binding = 0 : i32,
                           %arg1: !spv.ptr<!spv.struct<!spv.array<12 x !spv.array<4 x f32>>>, StorageBuffer>
-                          {spirv.interface_var_abi = {binding = 1 : i32,
-                                                      descriptor_set = 0 : i32,
-                                                      storage_class = 12 : i32}},
+                          {spv.interface_var_abi = {binding = 1 : i32,
                           %arg2: !spv.ptr<!spv.struct<!spv.array<12 x !spv.array<4 x f32>>>, StorageBuffer>
-                          {spirv.interface_var_abi = {binding = 2 : i32,
-                                                      descriptor_set = 0 : i32,
-                                                      storage_class = 12 : i32}},
+                          {spv.interface_var_abi = {binding = 2 : i32,
                           %arg3: i32
-                          {spirv.interface_var_abi = {binding = 3 : i32,
-                                                      descriptor_set = 0 : i32,
-                                                      storage_class = 12 : i32}},
+                          {spv.interface_var_abi = {binding = 3 : i32,
                           %arg4: i32
-                          {spirv.interface_var_abi = {binding = 4 : i32,
-                                                      descriptor_set = 0 : i32,
-                                                      storage_class = 12 : i32}},
+                          {spv.interface_var_abi = {binding = 4 : i32,
                           %arg5: i32
-                          {spirv.interface_var_abi = {binding = 5 : i32,
-                                                      descriptor_set = 0 : i32,
-                                                      storage_class = 12 : i32}},
+                          {spv.interface_var_abi = {binding = 5 : i32,
                           %arg6: i32
-                          {spirv.interface_var_abi = {binding = 6 : i32,
-                                                      descriptor_set = 0 : i32,
-                                                      storage_class = 12 : i32}})
-  attributes  {spirv.entry_point_abi = {local_size = dense<[32, 1, 1]> : vector<3xi32>}} {
+                          {spv.interface_var_abi = {binding = 6 : i32,
+  attributes  {spv.entry_point_abi = {local_size = dense<[32, 1, 1]> : vector<3xi32>}} {
     // CHECK: [[ADDRESSARG6:%.*]] = spv._address_of [[VAR6]]
     // CHECK: [[CONST6:%.*]] = spv.constant 0 : i32
     // CHECK: [[ARG6PTR:%.*]] = spv.AccessChain [[ADDRESSARG6]]{{\[}}[[CONST6]]
     // CHECK: {{%.*}} = spv.Load "StorageBuffer" [[ARG6PTR]]
     // CHECK: [[ADDRESSARG5:%.*]] = spv._address_of [[VAR5]]
     // CHECK: [[CONST5:%.*]] = spv.constant 0 : i32
     // CHECK: [[ARG5PTR:%.*]] = spv.AccessChain [[ADDRESSARG5]]{{\[}}[[CONST5]]
     // CHECK: {{%.*}} = spv.Load "StorageBuffer" [[ARG5PTR]]

mlir/test/Dialect/SPIRV/Transforms/abi-simple.mlir

 // RUN: mlir-opt -spirv-lower-abi-attrs -verify-diagnostics %s -o - | FileCheck %s
 
 // CHECK-LABEL: spv.module
 spv.module "Logical" "GLSL450" {
   // CHECK-DAG:    spv.globalVariable [[VAR0:@.*]] bind(0, 0) : !spv.ptr<!spv.struct<f32 [0]>, StorageBuffer>
   // CHECK-DAG:    spv.globalVariable [[VAR1:@.*]] bind(0, 1) : !spv.ptr<!spv.struct<!spv.array<12 x f32 [4]> [0]>, StorageBuffer>
   // CHECK:    func [[FN:@.*]]()
   func @kernel_1(%arg0: f32
-                {spirv.interface_var_abi = {binding = 0 : i32,
-                                            descriptor_set = 0 : i32,
-                                            storage_class = 12 : i32}},
+                {spv.interface_var_abi = {binding = 0 : i32,
                  %arg1: !spv.ptr<!spv.struct<!spv.array<12 x f32>>, StorageBuffer>
-                 {spirv.interface_var_abi = {binding = 1 : i32,
-                                             descriptor_set = 0 : i32,
-                                             storage_class = 12 : i32}})
-  attributes  {spirv.entry_point_abi = {local_size = dense<[32, 1, 1]> : vector<3xi32>}} {
+                 {spv.interface_var_abi = {binding = 1 : i32,
+  attributes  {spv.entry_point_abi = {local_size = dense<[32, 1, 1]> : vector<3xi32>}} {
     // CHECK: [[ARG1:%.*]] = spv._address_of [[VAR1]]
     // CHECK: [[ADDRESSARG0:%.*]] = spv._address_of [[VAR0]]
     // CHECK: [[CONST0:%.*]] = spv.constant 0 : i32
     // CHECK: [[ARG0PTR:%.*]] = spv.AccessChain [[ADDRESSARG0]]{{\[}}[[CONST0]]
     // CHECK: [[ARG0:%.*]] = spv.Load "StorageBuffer" [[ARG0PTR]]
     // CHECK: spv.Return
     spv.Return
   }
   // CHECK: spv.EntryPoint "GLCompute" [[FN]]
   // CHECK: spv.ExecutionMode [[FN]] "LocalSize", 32, 1, 1
 } attributes {capabilities = ["Shader"], extensions = ["SPV_KHR_storage_buffer_storage_class"]}

mlir/test/Dialect/SPIRV/target-and-abi.mlir

+// RUN: mlir-opt -split-input-file -verify-diagnostics %s | FileCheck %s
+
+// expected-error @+1 {{found unsupported 'spv.something' attribute on operation}}
+func @unknown_attr() attributes {
+  spv.something = 64
+} { return }
+
+// -----
+
+//===----------------------------------------------------------------------===//
+// spv.entry_point_abi
+//===----------------------------------------------------------------------===//
+
+// expected-error @+1 {{'spv.entry_point_abi' attribute must be a dictionary attribute containing one integer elements attribute: 'local_size'}}
+func @spv_entry_point() attributes {
+  spv.entry_point_abi = 64
+} { return }
+
+// -----
+
+// expected-error @+1 {{'spv.entry_point_abi' attribute must be a dictionary attribute containing one integer elements attribute: 'local_size'}}
+func @spv_entry_point() attributes {
+  spv.entry_point_abi = {local_size = 64}
+} { return }
+
+// -----
+
+func @spv_entry_point() attributes {
+  // CHECK: {spv.entry_point_abi = {local_size = dense<[64, 1, 1]> : vector<3xi32>}}
+  spv.entry_point_abi = {local_size = dense<[64, 1, 1]>: vector<3xi32>}
+} { return }
+
+// -----
+
+//===----------------------------------------------------------------------===//
+// spv.interface_var_abi
+//===----------------------------------------------------------------------===//
+
+// expected-error @+1 {{'spv.interface_var_abi' attribute must be a dictionary attribute containing three integer attribute: 'descriptor_set', 'binding', and 'storage_class'}}
+func @interface_var(
+  %arg0 : f32 {spv.interface_var_abi = 64}
+) { return }
+
+// -----
+
+// expected-error @+1 {{'spv.interface_var_abi' attribute must be a dictionary attribute containing three integer attribute: 'descriptor_set', 'binding', and 'storage_class'}}
+func @interface_var(
+  %arg0 : f32 {spv.interface_var_abi = {binding = 0: i32}}
+) { return }
+
+// -----
+
+// CHECK: {spv.interface_var_abi = {binding = 0 : i32, descriptor_set = 0 : i32, storage_class = 12 : i32}}
+func @interface_var(
+  %arg0 : f32 {spv.interface_var_abi = {binding = 0 : i32,
+                                        descriptor_set = 0 : i32,
+                                        storage_class = 12 : i32}}
+) { return }
+
+// -----
+
+// expected-error @+1 {{'spv.interface_var_abi' attribute must be a dictionary attribute containing three integer attribute: 'descriptor_set', 'binding', and 'storage_class'}}
+func @interface_var() -> (f32 {spv.interface_var_abi = 64})
+{
+  %0 = constant 10.0 : f32
+  return %0: f32
+}
+
+// -----
+
+// expected-error @+1 {{'spv.interface_var_abi' attribute must be a dictionary attribute containing three integer attribute: 'descriptor_set', 'binding', and 'storage_class'}}
+func @interface_var() -> (f32 {spv.interface_var_abi = {binding = 0: i32}})
+{
+  %0 = constant 10.0 : f32
+  return %0: f32
+}
+
+// -----
+
+// CHECK: {spv.interface_var_abi = {binding = 0 : i32, descriptor_set = 0 : i32, storage_class = 12 : i32}}
+func @interface_var() -> (f32 {spv.interface_var_abi = {
+    binding = 0 : i32, descriptor_set = 0 : i32, storage_class = 12 : i32}})
+{
+  %0 = constant 10.0 : f32
+  return %0: f32
+}