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.

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

let a user select preferred/unpreferred capabilities in a list of enabling capabilities #81476

Conversation

VyacheslavLevytskyy
Copy link
Contributor

@VyacheslavLevytskyy VyacheslavLevytskyy commented Feb 12, 2024

By SPIR-V specification: "If an instruction, enumerant, or other feature specifies multiple enabling capabilities, only one such
capability needs to be declared to use the feature."

However, one capability may be preferred over another. One important case is Shader capability that may not be supported by a backend, but always is inserted if "OpDecorate SpecId" is found, because Enabling Capabilities for the latter is the list of Shader and Kernel, where Shader is coming first and thus always selected as the first available option.

In this PR we address the problem by keeping current behaviour of selecting the first option among enabling capabilities as is, but giving a user a way to filter capabilities during the selection process via a newly introduced "--avoid-spirv-capabilities" command line option. This option is to avoid selection of certain capabilities if there are other available enabling capabilities.

This PR is changing also existing pruneCapabilities() function. It doesn't remove capability from module requirement anymore, but only adds implicitly required capabilities recursively, so its name is changed accordingly. This change fixes the present bug in collecting required by a module capabilities. Before the change, introduced by this PR, pruneCapabilities() function has been removing, for example, Kernel capability from required by a module, because Kernel is initially required and the second time it was needed pruneCapabilities() removed it by mistake.

@VyacheslavLevytskyy VyacheslavLevytskyy marked this pull request as ready for review February 12, 2024 14:29
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 12, 2024

@llvm/pr-subscribers-backend-spir-v

Author: Vyacheslav Levytskyy (VyacheslavLevytskyy)

Changes

By SPIR-V specification: "If an instruction, enumerant, or other feature specifies multiple enabling capabilities, only one such
capability needs to be declared to use the feature."

However, one capability may be preferred over another. One important case is Shader capability that may not be supported by a backend, but always is inserted if "OpDecorate SpecId" is found, because Enabling Capabilities for the latter is the list of Shader and Kernel, where Shader is coming first and thus always selected as the first available option.

In this PR we address the problem by keeping current behaviour of selecting the first option among enabling capabilities as is, but giving a user a way to filter capabilities during the selection process via a newly introduced "--avoid-spirv-capabilities" command line option. This option is to avoid selection of certain capabilities if there are other available options.

This PR is changing also existing pruneCapabilities() function by removing its capability deletion functionality and renaming it according to its role. The reason is that deletion of capabilities causes bugs by removing mandatory/required capabilities but doesn't introduce new or required functionality. The correspondent test case is updated.


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

3 Files Affected:

  • (modified) llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp (+39-7)
  • (modified) llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.h (+3-3)
  • (modified) llvm/test/CodeGen/SPIRV/transcoding/spec_const.ll (+3-3)
diff --git a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
index a18aae1761c834..9f14ea5dbe19b3 100644
--- a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
@@ -35,6 +35,22 @@ static cl::opt<bool>
                 cl::desc("Dump MIR with SPIR-V dependencies info"),
                 cl::Optional, cl::init(false));
 
+static cl::list<SPIRV::Capability::Capability>
+    AvoidCapabilities("avoid-spirv-capabilities",
+                      cl::desc("SPIR-V capabilities to avoid if there are "
+                               "other options enabling a feature"),
+                      cl::ZeroOrMore, cl::Hidden,
+                      cl::values(clEnumValN(SPIRV::Capability::Shader, "Shader",
+                                            "SPIR-V Shader capability")));
+// Use sets instead of cl::list to check "if contains" condition
+struct AvoidCapabilitiesSet {
+  SmallSet<SPIRV::Capability::Capability, 4> S;
+  AvoidCapabilitiesSet() {
+    for (auto Cap : AvoidCapabilities)
+      S.insert(Cap);
+  }
+};
+
 char llvm::SPIRVModuleAnalysis::ID = 0;
 
 namespace llvm {
@@ -58,6 +74,8 @@ static SPIRV::Requirements
 getSymbolicOperandRequirements(SPIRV::OperandCategory::OperandCategory Category,
                                unsigned i, const SPIRVSubtarget &ST,
                                SPIRV::RequirementHandler &Reqs) {
+  static AvoidCapabilitiesSet
+      AvoidCaps; // contains capabilities to avoid if there is another option
   unsigned ReqMinVer = getSymbolicOperandMinVersion(Category, i);
   unsigned ReqMaxVer = getSymbolicOperandMaxVersion(Category, i);
   unsigned TargetVer = ST.getSPIRVVersion();
@@ -72,9 +90,26 @@ getSymbolicOperandRequirements(SPIRV::OperandCategory::OperandCategory Category,
       return {false, {}, {}, 0, 0};
     }
   } else if (MinVerOK && MaxVerOK) {
-    for (auto Cap : ReqCaps) { // Only need 1 of the capabilities to work.
+    if (ReqCaps.size() == 1) {
+      auto Cap = ReqCaps[0];
       if (Reqs.isCapabilityAvailable(Cap))
         return {true, {Cap}, {}, ReqMinVer, ReqMaxVer};
+    } else {
+      // By SPIR-V specification: "If an instruction, enumerant, or other
+      // feature specifies multiple enabling capabilities, only one such
+      // capability needs to be declared to use the feature." However, one
+      // capability may be preferred over another. We use command line
+      // argument(s) and AvoidCapabilities to avoid selection of certain
+      // capabilities if there are other options.
+      CapabilityList UseCaps;
+      for (auto Cap : ReqCaps)
+        if (Reqs.isCapabilityAvailable(Cap))
+          UseCaps.push_back(Cap);
+      for (size_t i = 0, Sz = UseCaps.size(); i < Sz; ++i) {
+        auto Cap = UseCaps[i];
+        if (i == Sz - 1 || !AvoidCaps.S.contains(Cap))
+          return {true, {Cap}, {}, ReqMinVer, ReqMaxVer};
+      }
     }
   }
   // If there are no capabilities, or we can't satisfy the version or
@@ -432,16 +467,13 @@ void SPIRV::RequirementHandler::getAndAddRequirements(
   addRequirements(getSymbolicOperandRequirements(Category, i, ST, *this));
 }
 
-void SPIRV::RequirementHandler::pruneCapabilities(
+void SPIRV::RequirementHandler::recursiveAddCapabilities(
     const CapabilityList &ToPrune) {
   for (const auto &Cap : ToPrune) {
     AllCaps.insert(Cap);
-    auto FoundIndex = llvm::find(MinimalCaps, Cap);
-    if (FoundIndex != MinimalCaps.end())
-      MinimalCaps.erase(FoundIndex);
     CapabilityList ImplicitDecls =
         getSymbolicOperandCapabilities(OperandCategory::CapabilityOperand, Cap);
-    pruneCapabilities(ImplicitDecls);
+    recursiveAddCapabilities(ImplicitDecls);
   }
 }
 
@@ -452,7 +484,7 @@ void SPIRV::RequirementHandler::addCapabilities(const CapabilityList &ToAdd) {
       continue;
     CapabilityList ImplicitDecls =
         getSymbolicOperandCapabilities(OperandCategory::CapabilityOperand, Cap);
-    pruneCapabilities(ImplicitDecls);
+    recursiveAddCapabilities(ImplicitDecls);
     MinimalCaps.push_back(Cap);
   }
 }
diff --git a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.h b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.h
index b05526b06e7da7..708384fc55f525 100644
--- a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.h
+++ b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.h
@@ -71,9 +71,9 @@ struct RequirementHandler {
   SmallSet<Extension::Extension, 4> AllExtensions;
   unsigned MinVersion; // 0 if no min version is defined.
   unsigned MaxVersion; // 0 if no max version is defined.
-  // Remove a list of capabilities from dedupedCaps and add them to AllCaps,
-  // recursing through their implicitly declared capabilities too.
-  void pruneCapabilities(const CapabilityList &ToPrune);
+  // Add capabilities to AllCaps, recursing through their implicitly declared
+  // capabilities too.
+  void recursiveAddCapabilities(const CapabilityList &ToPrune);
 
   void initAvailableCapabilitiesForOpenCL(const SPIRVSubtarget &ST);
   void initAvailableCapabilitiesForVulkan(const SPIRVSubtarget &ST);
diff --git a/llvm/test/CodeGen/SPIRV/transcoding/spec_const.ll b/llvm/test/CodeGen/SPIRV/transcoding/spec_const.ll
index c47dccb35e14d5..8ce76534c50db5 100644
--- a/llvm/test/CodeGen/SPIRV/transcoding/spec_const.ll
+++ b/llvm/test/CodeGen/SPIRV/transcoding/spec_const.ll
@@ -1,9 +1,9 @@
-; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s --check-prefix=CHECK-SPIRV
-; XFAIL: *
+; RUN: llc -O0 -mtriple=spirv32-unknown-unknown --avoid-spirv-capabilities=Shader %s -o - | FileCheck %s --check-prefix=CHECK-SPIRV
+; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv32-unknown-unknown --avoid-spirv-capabilities=Shader %s -o - -filetype=obj | spirv-val %}
 
+; CHECK-SPIRV-DAG: OpCapability Kernel
 ; CHECK-SPIRV-NOT: OpCapability Matrix
 ; CHECK-SPIRV-NOT: OpCapability Shader
-; CHECK-SPIRV:     OpCapability Kernel
 
 ; CHECK-SPIRV-DAG: OpDecorate %[[#SC0:]] SpecId 0
 ; CHECK-SPIRV-DAG: OpDecorate %[[#SC1:]] SpecId 1

@VyacheslavLevytskyy VyacheslavLevytskyy changed the title let user select among alternative enabling capabilities let a user select preferred/unpreferred capabilities in a list of enabling capabilities Feb 12, 2024
@VyacheslavLevytskyy VyacheslavLevytskyy merged commit dfb9bf3 into llvm:main Feb 15, 2024
9 checks passed
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.

None yet

3 participants