Skip to content

Conversation

@IgWod-IMG
Copy link
Contributor

This patch adds an assert to check if the result of getConstantInt is non-null. Previously the code failed with Segmentation Fault if getConstantInt failed to look up the value. This primarily occurred when the value was defined as OpSpecConstant rather than OpConstant.

This patch adds an assert to check if the result of getConstantInt
is non-null. Previously the code failed with Segmentation Fault if
getConstantInt failed to look up the value. This primarily occurred
when the value was defined as OpSpecConstant rather than OpConstant.
@llvmbot
Copy link
Member

llvmbot commented May 19, 2025

@llvm/pr-subscribers-mlir-spirv

@llvm/pr-subscribers-mlir

Author: Igor Wodiany (IgWod-IMG)

Changes

This patch adds an assert to check if the result of getConstantInt is non-null. Previously the code failed with Segmentation Fault if getConstantInt failed to look up the value. This primarily occurred when the value was defined as OpSpecConstant rather than OpConstant.


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

1 Files Affected:

  • (modified) mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp (+11-4)
diff --git a/mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp b/mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp
index 7afd6e9b25b77..9008d88866f40 100644
--- a/mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp
+++ b/mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp
@@ -1061,12 +1061,19 @@ LogicalResult spirv::Deserializer::processCooperativeMatrixTypeKHR(
            << operands[2];
   }
 
-  unsigned rows = getConstantInt(operands[3]).getInt();
-  unsigned columns = getConstantInt(operands[4]).getInt();
+  IntegerAttr rowsAttr = getConstantInt(operands[3]);
+  assert(rowsAttr);
+  unsigned rows = rowsAttr.getInt();
+
+  IntegerAttr columnsAttr = getConstantInt(operands[4]);
+  assert(columnsAttr);
+  unsigned columns = columnsAttr.getInt();
+
+  IntegerAttr useAttr = getConstantInt(operands[5]);
+  assert(useAttr);
 
   std::optional<spirv::CooperativeMatrixUseKHR> use =
-      spirv::symbolizeCooperativeMatrixUseKHR(
-          getConstantInt(operands[5]).getInt());
+      spirv::symbolizeCooperativeMatrixUseKHR(useAttr.getInt());
   if (!use) {
     return emitError(
                unknownLoc,

Copy link
Member

@kuhar kuhar left a comment

Choose a reason for hiding this comment

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

LGTM % readability

@IgWod-IMG IgWod-IMG force-pushed the img_get-const-int-assert branch from 2526c7a to 19d5056 Compare June 4, 2025 09:43
@IgWod-IMG
Copy link
Contributor Author

I just re-pushed the same commit to re-kick CI. There were some flang failures that looked unrelated, but I just want to double check it.

@IgWod-IMG IgWod-IMG merged commit 3ce3281 into llvm:main Jun 4, 2025
11 checks passed
@IgWod-IMG IgWod-IMG deleted the img_get-const-int-assert branch June 4, 2025 12:15
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.

4 participants