[mlir][spirv] NFC: Move shader ABI attributes to a new file

[antiagainst]

Summary

This allows us to include the definitions of these attributes in
other files without pulling in all dependencies for lowering.

[antiagainst]

Update diff

[merge_guards_bot]

Unit tests: pass. 61168 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

[merge_guards_bot]

Unit tests: pass. 61168 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]

Just wondering why the refactoring?

[antiagainst]

In D72054#1800648, @mravishankar wrote:

Just wondering why the refactoring?

This is preparing for adding verification hooks for these attributes. The verification hooks are defined in the SPIR-V dialect class so we need to have a cleaner story for dependency. (I'll have a follow-up CL, which will make this clearer.)

[antiagainst]

Here is the following-up CL: https://reviews.llvm.org/D72062

mlir/include/mlir/Dialect/SPIRV/CMakeLists.txt

 set(LLVM_TARGET_DEFINITIONS SPIRVOps.td)
 mlir_tablegen(SPIRVSerialization.inc -gen-spirv-serialization)
 add_public_tablegen_target(MLIRSPIRVSerializationGen)
 
 set(LLVM_TARGET_DEFINITIONS SPIRVBase.td)
 mlir_tablegen(SPIRVOpUtils.inc -gen-spirv-op-utils)
 add_public_tablegen_target(MLIRSPIRVOpUtilsGen)
 
-set(LLVM_TARGET_DEFINITIONS SPIRVLowering.td)
-mlir_tablegen(SPIRVLowering.h.inc -gen-struct-attr-decls)
-mlir_tablegen(SPIRVLowering.cpp.inc -gen-struct-attr-defs)
-add_public_tablegen_target(MLIRSPIRVLoweringStructGen)
+set(LLVM_TARGET_DEFINITIONS TargetAndABI.td)
+mlir_tablegen(TargetAndABI.h.inc -gen-struct-attr-decls)
+mlir_tablegen(TargetAndABI.cpp.inc -gen-struct-attr-defs)
+add_public_tablegen_target(MLIRSPIRVTargetAndABIIncGen)

mlir/include/mlir/Dialect/SPIRV/SPIRVLowering.h

 //===- SPIRVLowering.h - SPIR-V lowering utilities  -------------*- 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
 //
 //===----------------------------------------------------------------------===//
 //
 // Defines utilities to use while targeting SPIR-V dialect.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef MLIR_DIALECT_SPIRV_SPIRVLOWERING_H
 #define MLIR_DIALECT_SPIRV_SPIRVLOWERING_H
 
-#include "mlir/Dialect/SPIRV/SPIRVOps.h"
-#include "mlir/IR/Attributes.h"
-#include "mlir/Support/StringExtras.h"
+#include "mlir/Dialect/SPIRV/TargetAndABI.h"
 #include "mlir/Transforms/DialectConversion.h"
-#include "llvm/ADT/SetVector.h"
 
 namespace mlir {
 
 /// Type conversion from standard types to SPIR-V types for shader interface.
 ///
 /// For composite types, this converter additionally performs type wrapping to
 /// satisfy shader interface requirements: shader interface types must be
 /// pointers to structs.
SPIRVOpLowering(MLIRContext *context, SPIRVTypeConverter &typeConverter,
                   PatternBenefit benefit = 1)
       : OpConversionPattern<SourceOp>(context, benefit),
         typeConverter(typeConverter) {}
 
 protected:
   SPIRVTypeConverter &typeConverter;
 };
 
-#include "mlir/Dialect/SPIRV/SPIRVLowering.h.inc"
-
 namespace spirv {
+enum class BuiltIn : uint32_t;
+
 /// Returns a value that represents a builtin variable value within the SPIR-V
 /// module.
-Value getBuiltinVariableValue(Operation *op, spirv::BuiltIn builtin,
+Value getBuiltinVariableValue(Operation *op, BuiltIn builtin,
                               OpBuilder &builder);
 
-/// Attribute name for specifying argument ABI information.
-StringRef getInterfaceVarABIAttrName();
-
-/// Get the InterfaceVarABIAttr given its fields.
-InterfaceVarABIAttr getInterfaceVarABIAttr(unsigned descriptorSet,
-                                           unsigned binding,
-                                           spirv::StorageClass storageClass,
-                                           MLIRContext *context);
-
-/// Attribute name for specifying entry point information.
-StringRef getEntryPointABIAttrName();
-
-/// Get the EntryPointABIAttr given its fields.
-EntryPointABIAttr getEntryPointABIAttr(ArrayRef<int32_t> localSize,
-                                       MLIRContext *context);
-
 /// Sets the InterfaceVarABIAttr and EntryPointABIAttr for a function and its
-/// arguments
-LogicalResult setABIAttrs(FuncOp funcOp,
-                          spirv::EntryPointABIAttr entryPointInfo,
-                          ArrayRef<spirv::InterfaceVarABIAttr> argABIInfo);
-
+/// arguments.
+LogicalResult setABIAttrs(FuncOp funcOp, EntryPointABIAttr entryPointInfo,
+                          ArrayRef<InterfaceVarABIAttr> argABIInfo);
 } // namespace spirv
 } // namespace mlir
 
 #endif // MLIR_DIALECT_SPIRV_SPIRVLOWERING_H

mlir/include/mlir/Dialect/SPIRV/TargetAndABI.h

+//===- TargetAndABI.h - SPIR-V target and ABI utilities  --------*- 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 utilities for SPIR-V target and shader interface ABI.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef MLIR_DIALECT_SPIRV_TARGETANDABI_H
+#define MLIR_DIALECT_SPIRV_TARGETANDABI_H
+
+#include "mlir/IR/Attributes.h"
+#include "mlir/Support/LLVM.h"
+
+namespace mlir {
+class OpBuilder;
+class Operation;
+class Value;
+
+// Pull in SPIR-V attribute definitions.
+#include "mlir/Dialect/SPIRV/TargetAndABI.h.inc"
+
+namespace spirv {
+enum class StorageClass : uint32_t;
+
+/// Attribute name for specifying argument ABI information.
+StringRef getInterfaceVarABIAttrName();
+
+/// Get the InterfaceVarABIAttr given its fields.
+InterfaceVarABIAttr getInterfaceVarABIAttr(unsigned descriptorSet,
+                                           unsigned binding,
+                                           StorageClass storageClass,
+                                           MLIRContext *context);
+
+/// Attribute name for specifying entry point information.
+StringRef getEntryPointABIAttrName();
+
+/// Get the EntryPointABIAttr given its fields.
+EntryPointABIAttr getEntryPointABIAttr(ArrayRef<int32_t> localSize,
+                                       MLIRContext *context);
+} // namespace spirv
+} // namespace mlir
+
+#endif // MLIR_DIALECT_SPIRV_TARGETANDABI_H

mlir/lib/Dialect/SPIRV/CMakeLists.txt

 set(LLVM_TARGET_DEFINITIONS SPIRVCanonicalization.td)
 mlir_tablegen(SPIRVCanonicalization.inc -gen-rewriters)
 add_public_tablegen_target(MLIRSPIRVCanonicalizationIncGen)
 
 add_llvm_library(MLIRSPIRV
   DialectRegistration.cpp
   LayoutUtils.cpp
   SPIRVDialect.cpp
   SPIRVOps.cpp
   SPIRVLowering.cpp
   SPIRVTypes.cpp
+  TargetAndABI.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${MLIR_MAIN_INCLUDE_DIR}/mlir/SPIRV
   )
 
 add_dependencies(MLIRSPIRV
   MLIRSPIRVAvailabilityIncGen
   MLIRSPIRVCanonicalizationIncGen
   MLIRSPIRVEnumsIncGen
-  MLIRSPIRVLoweringStructGen
   MLIRSPIRVOpsIncGen
-  MLIRSPIRVOpUtilsGen)
+  MLIRSPIRVOpUtilsGen
+  MLIRSPIRVTargetAndABIIncGen)
 
 target_link_libraries(MLIRSPIRV
   MLIRIR
   MLIRParser
   MLIRSupport
   MLIRTransforms)
 
 add_subdirectory(Serialization)
 add_subdirectory(Transforms)

mlir/lib/Dialect/SPIRV/SPIRVLowering.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
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements utilities used to lower to SPIR-V dialect.
 //
 //===----------------------------------------------------------------------===//
+
 #include "mlir/Dialect/SPIRV/SPIRVLowering.h"
 #include "mlir/Dialect/SPIRV/LayoutUtils.h"
 #include "mlir/Dialect/SPIRV/SPIRVDialect.h"
+#include "mlir/Dialect/SPIRV/SPIRVOps.h"
 #include "llvm/ADT/Sequence.h"
 
 using namespace mlir;
 
 //===----------------------------------------------------------------------===//
-// Attributes for ABI
-//===----------------------------------------------------------------------===//
-
-// Pull in the attributes needed for lowering.
-namespace mlir {
-#include "mlir/Dialect/SPIRV/SPIRVLowering.cpp.inc"
-}
-
-StringRef mlir::spirv::getInterfaceVarABIAttrName() {
-  return "spirv.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";
-}
-
-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);
-}
-
-//===----------------------------------------------------------------------===//
 // Type Conversion
 //===----------------------------------------------------------------------===//
 
 Type SPIRVTypeConverter::getIndexType(MLIRContext *context) {
   // Convert to 32-bit integers for now. Might need a way to control this in
   // future.
   // TODO(ravishankarm): It is probably better to make it 64-bit integers. To
   // this some support is needed in SPIR-V dialect for Conversion

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";
+}
+
+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";
+}
+
+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/lib/Dialect/SPIRV/Transforms/LowerABIAttributesPass.cpp

 // for the functions in the generated SPIR-V module.
 //
 //===----------------------------------------------------------------------===//
 
 #include "mlir/Dialect/SPIRV/LayoutUtils.h"
 #include "mlir/Dialect/SPIRV/Passes.h"
 #include "mlir/Dialect/SPIRV/SPIRVDialect.h"
 #include "mlir/Dialect/SPIRV/SPIRVLowering.h"
+#include "mlir/Dialect/SPIRV/SPIRVOps.h"
 #include "mlir/Dialect/StandardOps/Ops.h"
 #include "mlir/Transforms/DialectConversion.h"
+#include "llvm/ADT/SetVector.h"
 
 using namespace mlir;
 
 /// Checks if the `type` is a scalar or vector type. It is assumed that they are
 /// valid for SPIR-V dialect already.
 static bool isScalarOrVectorType(Type type) {
   return spirv::SPIRVDialect::isValidScalarType(type) || type.isa<VectorType>();
 }