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

fix generation of unnecessary OpExecutionMode records #81839

Merged

Conversation

VyacheslavLevytskyy
Copy link
Contributor

@VyacheslavLevytskyy VyacheslavLevytskyy commented Feb 15, 2024

SPIRV-V Backend generates unnecessary OpExecutionMode records, putting into the id's which are not the Entry Point operands of an OpEntryPoint (ref: #81753). This PR is to fix the issue.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 15, 2024

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

Author: Vyacheslav Levytskyy (VyacheslavLevytskyy)

Changes

SPIRV-V Backend generates unnecessary OpExecutionMode records, putting into the 's which are not the Entry Point operands of an OpEntryPoint (ref: #81753). This PR is to fix the issue.


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

4 Files Affected:

  • (modified) llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp (+3-1)
  • (modified) llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp (-14)
  • (modified) llvm/lib/Target/SPIRV/SPIRVUtils.cpp (+14)
  • (modified) llvm/lib/Target/SPIRV/SPIRVUtils.h (+3)
diff --git a/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp b/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp
index 27da0f21f1571d..1fbf3c3e11aedc 100644
--- a/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp
@@ -450,7 +450,9 @@ void SPIRVAsmPrinter::outputExecutionMode(const Module &M) {
   }
   for (auto FI = M.begin(), E = M.end(); FI != E; ++FI) {
     const Function &F = *FI;
-    if (F.isDeclaration())
+    // Only operands of OpEntryPoint instructions are allowed to be
+    // <Entry Point> operands of OpExecutionMode
+    if (F.isDeclaration() || !isEntryPoint(F))
       continue;
     Register FReg = MAI->getFuncReg(&F);
     assert(FReg.isValid());
diff --git a/llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp b/llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp
index 8ac498e1556bec..4711cf67ffab6e 100644
--- a/llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp
@@ -181,20 +181,6 @@ static SPIRVType *getArgSPIRVType(const Function &F, unsigned ArgIdx,
                                                ArgAccessQual);
 }
 
-static bool isEntryPoint(const Function &F) {
-  // OpenCL handling: any function with the SPIR_KERNEL
-  // calling convention will be a potential entry point.
-  if (F.getCallingConv() == CallingConv::SPIR_KERNEL)
-    return true;
-
-  // HLSL handling: special attribute are emitted from the
-  // front-end.
-  if (F.getFnAttribute("hlsl.shader").isValid())
-    return true;
-
-  return false;
-}
-
 static SPIRV::ExecutionModel::ExecutionModel
 getExecutionModel(const SPIRVSubtarget &STI, const Function &F) {
   if (STI.isOpenCLEnv())
diff --git a/llvm/lib/Target/SPIRV/SPIRVUtils.cpp b/llvm/lib/Target/SPIRV/SPIRVUtils.cpp
index d4f7d8e89af5e4..db5ae4e8c56c39 100644
--- a/llvm/lib/Target/SPIRV/SPIRVUtils.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVUtils.cpp
@@ -348,4 +348,18 @@ bool isSpecialOpaqueType(const Type *Ty) {
 
   return false;
 }
+
+bool isEntryPoint(const Function &F) {
+  // OpenCL handling: any function with the SPIR_KERNEL
+  // calling convention will be a potential entry point.
+  if (F.getCallingConv() == CallingConv::SPIR_KERNEL)
+    return true;
+
+  // HLSL handling: special attribute are emitted from the
+  // front-end.
+  if (F.getFnAttribute("hlsl.shader").isValid())
+    return true;
+
+  return false;
+}
 } // namespace llvm
diff --git a/llvm/lib/Target/SPIRV/SPIRVUtils.h b/llvm/lib/Target/SPIRV/SPIRVUtils.h
index 60742e2f272808..a33dc02f854f58 100644
--- a/llvm/lib/Target/SPIRV/SPIRVUtils.h
+++ b/llvm/lib/Target/SPIRV/SPIRVUtils.h
@@ -97,5 +97,8 @@ bool hasBuiltinTypePrefix(StringRef Name);
 
 // Check if given LLVM type is a special opaque builtin type.
 bool isSpecialOpaqueType(const Type *Ty);
+
+// Check if the function is an SPIR-V entry point
+bool isEntryPoint(const Function &F);
 } // namespace llvm
 #endif // LLVM_LIB_TARGET_SPIRV_SPIRVUTILS_H

@VyacheslavLevytskyy VyacheslavLevytskyy merged commit 8e8f9c0 into llvm:main Feb 19, 2024
3 of 5 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

4 participants