Skip to content

[mlir][gpu] Clean GPU Passes.h from external SPIRV includes #71331

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.

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

Merged
merged 1 commit into from
Nov 6, 2023

Conversation

fabianmcg
Copy link
Contributor

Removes the SPIRVAttributes.h header from GPU/Transforms/Passes.h

Removes the `SPIRVAttributes.h` header from `gpu/Passes.h`.
@llvmbot
Copy link
Member

llvmbot commented Nov 5, 2023

@llvm/pr-subscribers-mlir-gpu

@llvm/pr-subscribers-mlir

Author: Fabian Mora (fabianmcg)

Changes

Removes the SPIRVAttributes.h header from GPU/Transforms/Passes.h


Full diff: https://github.com/llvm/llvm-project/pull/71331.diff

4 Files Affected:

  • (modified) mlir/include/mlir/Dialect/GPU/Transforms/Passes.h (-1)
  • (modified) mlir/include/mlir/Dialect/GPU/Transforms/Passes.td (+1-1)
  • (modified) mlir/lib/Dialect/GPU/Transforms/SPIRVAttachTarget.cpp (+3)
  • (modified) mlir/test/Dialect/GPU/spirv-attach-targets.mlir (+6-6)
diff --git a/mlir/include/mlir/Dialect/GPU/Transforms/Passes.h b/mlir/include/mlir/Dialect/GPU/Transforms/Passes.h
index 42fa46b0a57bdee..2a891a7d24f809a 100644
--- a/mlir/include/mlir/Dialect/GPU/Transforms/Passes.h
+++ b/mlir/include/mlir/Dialect/GPU/Transforms/Passes.h
@@ -15,7 +15,6 @@
 
 #include "Utils.h"
 #include "mlir/Dialect/GPU/IR/GPUDialect.h"
-#include "mlir/Dialect/SPIRV/IR/SPIRVAttributes.h"
 #include "mlir/Pass/Pass.h"
 #include <optional>
 
diff --git a/mlir/include/mlir/Dialect/GPU/Transforms/Passes.td b/mlir/include/mlir/Dialect/GPU/Transforms/Passes.td
index b22d26d49dbdb0e..059893127295bf5 100644
--- a/mlir/include/mlir/Dialect/GPU/Transforms/Passes.td
+++ b/mlir/include/mlir/Dialect/GPU/Transforms/Passes.td
@@ -227,7 +227,7 @@ def GpuSPIRVAttachTarget: Pass<"spirv-attach-target", ""> {
            /*default=*/ "\"Unknown\"",
            "Device Type">,
     Option<"deviceId", "device_id", "uint32_t",
-           /*default=*/ "mlir::spirv::TargetEnvAttr::kUnknownDeviceID",
+           /*default=*/ "",
            "Device ID">,
   ];
 }
diff --git a/mlir/lib/Dialect/GPU/Transforms/SPIRVAttachTarget.cpp b/mlir/lib/Dialect/GPU/Transforms/SPIRVAttachTarget.cpp
index eece62b9c6cb9c8..a099a44f3dc6aa6 100644
--- a/mlir/lib/Dialect/GPU/Transforms/SPIRVAttachTarget.cpp
+++ b/mlir/lib/Dialect/GPU/Transforms/SPIRVAttachTarget.cpp
@@ -57,6 +57,9 @@ void SPIRVAttachTarget::runOnOperation() {
   auto deviceTypeSymbol = symbolizeDeviceType(deviceType);
   if (!deviceTypeSymbol)
     return signalPassFailure();
+  // Set the default device ID if none was given
+  if (!deviceId.hasValue())
+    deviceId = mlir::spirv::TargetEnvAttr::kUnknownDeviceID;
 
   Version version = versionSymbol.value();
   SmallVector<Capability, 4> capabilities;
diff --git a/mlir/test/Dialect/GPU/spirv-attach-targets.mlir b/mlir/test/Dialect/GPU/spirv-attach-targets.mlir
index 2ab748834e49fac..766a1bae8abc239 100644
--- a/mlir/test/Dialect/GPU/spirv-attach-targets.mlir
+++ b/mlir/test/Dialect/GPU/spirv-attach-targets.mlir
@@ -1,16 +1,16 @@
-// RUN: mlir-opt %s --spirv-attach-target='module=spirv.* ver=v1.0 caps=Kernel' | FileCheck %s
-// RUN: mlir-opt %s --spirv-attach-target='module=spirv_warm.* ver=v1.0 caps=Kernel' | FileCheck %s --check-prefix=CHECK_WARM
+// RUN: mlir-opt %s --spirv-attach-target='module=spirv.* ver=v1.0 caps=Kernel vendor=Intel device_type=Other' | FileCheck %s
+// RUN: mlir-opt %s --spirv-attach-target='module=spirv_warm.* ver=v1.0 caps=Kernel vendor=Intel device_type=Other device_id=0' | FileCheck %s --check-prefix=CHECK_WARM
 
 module attributes {gpu.container_module} {
-//      CHECK: @spirv_hot_module [#spirv.target_env<#spirv.vce<v1.0, [Kernel], []>, #spirv.resource_limits<>>]
+//      CHECK: @spirv_hot_module [#spirv.target_env<#spirv.vce<v1.0, [Kernel], []>, Intel:Other, #spirv.resource_limits<>>]
 // CHECK_WARM: @spirv_hot_module {
 gpu.module @spirv_hot_module {
 }
-//      CHECK: @spirv_warm_module [#spirv.target_env<#spirv.vce<v1.0, [Kernel], []>, #spirv.resource_limits<>>]
-// CHECK_WARM: @spirv_warm_module [#spirv.target_env<#spirv.vce<v1.0, [Kernel], []>, #spirv.resource_limits<>>]
+//      CHECK: @spirv_warm_module [#spirv.target_env<#spirv.vce<v1.0, [Kernel], []>, Intel:Other, #spirv.resource_limits<>>]
+// CHECK_WARM: @spirv_warm_module [#spirv.target_env<#spirv.vce<v1.0, [Kernel], []>, Intel:Other:0, #spirv.resource_limits<>>]
 gpu.module @spirv_warm_module {
 }
-//      CHECK: @spirv_cold_module [#spirv.target_env<#spirv.vce<v1.0, [Kernel], []>, #spirv.resource_limits<>>]
+//      CHECK: @spirv_cold_module [#spirv.target_env<#spirv.vce<v1.0, [Kernel], []>, Intel:Other, #spirv.resource_limits<>>]
 // CHECK_WARM: @spirv_cold_module {
 gpu.module @spirv_cold_module {
 }

Copy link
Member

@jpienaar jpienaar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jpienaar jpienaar merged commit 4263068 into llvm:main Nov 6, 2023
@fabianmcg fabianmcg deleted the fix branch November 6, 2023 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants