Skip to content

[SPIR-V] Discard some llvm intrinsics which we do not expect to actually represent code after lowering #110233

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

Conversation

VyacheslavLevytskyy
Copy link
Contributor

There are llvm intrinsics which we do not expect to actually represent code after lowering or which are not implemented yet but can be found in a customer's LLVM IR input. We do not want translation to crash when these llvm intrinsics are found, and this PR fixes the issue with translation crash for some known cases, aligned with Khronos Translator.

@llvmbot
Copy link
Member

llvmbot commented Sep 27, 2024

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

Author: Vyacheslav Levytskyy (VyacheslavLevytskyy)

Changes

There are llvm intrinsics which we do not expect to actually represent code after lowering or which are not implemented yet but can be found in a customer's LLVM IR input. We do not want translation to crash when these llvm intrinsics are found, and this PR fixes the issue with translation crash for some known cases, aligned with Khronos Translator.


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

3 Files Affected:

  • (modified) llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp (+9-1)
  • (modified) llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp (+17-1)
  • (added) llvm/test/CodeGen/SPIRV/llvm-intrinsics/ignore-llvm-intrinsic.ll (+25)
diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
index 415b5d99695f0d..1659e725c68f4a 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
@@ -189,6 +189,13 @@ bool isConvergenceIntrinsic(const Instruction *I) {
          II->getIntrinsicID() == Intrinsic::experimental_convergence_anchor;
 }
 
+bool expectIgnoredInIRTranslation(const Instruction *I) {
+  const auto *II = dyn_cast<IntrinsicInst>(I);
+  if (!II)
+    return false;
+  return II->getIntrinsicID() == Intrinsic::invariant_start;
+}
+
 bool allowEmitFakeUse(const Value *Arg) {
   if (const auto *II = dyn_cast<IntrinsicInst>(Arg))
     if (Function *F = II->getCalledFunction())
@@ -1563,7 +1570,8 @@ void SPIRVEmitIntrinsics::processInstrAfterVisit(Instruction *I,
       I->setOperand(OpNo, NewOp);
     }
   }
-  if (I->hasName() && !I->getType()->isAggregateType()) {
+  if (I->hasName() && !I->getType()->isAggregateType() &&
+      !expectIgnoredInIRTranslation(I)) {
     reportFatalOnTokenType(I);
     setInsertPointAfterDef(B, I);
     std::vector<Value *> Args = {I};
diff --git a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
index 2f7efbdc81f845..3c8ad196520087 100644
--- a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
@@ -745,6 +745,15 @@ bool SPIRVInstructionSelector::spvSelect(Register ResVReg,
   case TargetOpcode::G_UNMERGE_VALUES:
     return selectUnmergeValues(I);
 
+  // Discard gen opcodes for intrinsics which we do not expect to actually
+  // represent code after lowering or intrinsics which are not implemented but
+  // should not crash when found in a customer's LLVM IR input.
+  case TargetOpcode::G_TRAP:
+  case TargetOpcode::G_DEBUGTRAP:
+  case TargetOpcode::G_UBSANTRAP:
+  case TargetOpcode::DBG_LABEL:
+    return true;
+
   default:
     return false;
   }
@@ -2556,8 +2565,15 @@ bool SPIRVInstructionSelector::selectIntrinsic(Register ResVReg,
   }
   case Intrinsic::spv_step:
     return selectStep(ResVReg, ResType, I);
+  // Discard intrinsics which we do not expect to actually represent code after
+  // lowering or intrinsics which are not implemented but should not crash when
+  // found in a customer's LLVM IR input.
+  case Intrinsic::instrprof_increment:
+  case Intrinsic::instrprof_increment_step:
+  case Intrinsic::instrprof_value_profile:
+    break;
+  // Discard internal intrinsics.
   case Intrinsic::spv_value_md:
-    // ignore the intrinsic
     break;
   default: {
     std::string DiagMsg;
diff --git a/llvm/test/CodeGen/SPIRV/llvm-intrinsics/ignore-llvm-intrinsic.ll b/llvm/test/CodeGen/SPIRV/llvm-intrinsics/ignore-llvm-intrinsic.ll
new file mode 100644
index 00000000000000..a15a80754cd605
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/llvm-intrinsics/ignore-llvm-intrinsic.ll
@@ -0,0 +1,25 @@
+; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
+; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+
+; Ensure that these calls do not represent any code and don't cause a crash.
+; CHECK: OpFunction
+; CHECK-NEXT: OpFunctionParameter
+; CHECK-NEXT: OpLabel
+; CHECK-NEXT: OpReturn
+; CHECK-NEXT: OpFunctionEnd
+
+define spir_kernel void @foo(ptr %p) {
+entry:
+  call void @llvm.trap()
+  call void @llvm.debugtrap()
+  call void @llvm.ubsantrap(i8 100)
+
+  %r1 = call ptr @llvm.invariant.start.p0(i64 1024, ptr %p)
+  call void @llvm.invariant.end.p0(ptr %r1, i64 1024, ptr %p)
+
+  call void @llvm.instrprof.increment(ptr %p, i64 0, i32 1, i32 0)
+  call void @llvm.instrprof.increment.step(ptr %p, i64 0, i32 1, i32 0, i64 1)
+  call void @llvm.instrprof.value.profile(ptr %p, i64 0, i64 0, i32 1, i32 0)
+
+  ret void
+}

@VyacheslavLevytskyy VyacheslavLevytskyy merged commit c538d5c into llvm:main Oct 1, 2024
9 of 11 checks passed
Sterling-Augustine pushed a commit to Sterling-Augustine/llvm-project that referenced this pull request Oct 3, 2024
…lly represent code after lowering (llvm#110233)

There are llvm intrinsics which we do not expect to actually represent
code after lowering or which are not implemented yet but can be found in
a customer's LLVM IR input. We do not want translation to crash when
these llvm intrinsics are found, and this PR fixes the issue with
translation crash for some known cases, aligned with Khronos Translator.
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