[Mlir-commits] [mlir] [mlir][SPIR-V] Move symbol lookups into verifySymbolUses (PR #235042)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Fri Oct 23 10:58:06 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir-spirv
Author: Arseniy Obolenskiy (aobolensk)
<details>
<summary>Changes</summary>
Looking up symbols in verify() made verification quadratic
---
Full diff: https://github.com/llvm/llvm-project/pull/235042.diff
3 Files Affected:
- (modified) mlir/include/mlir/Dialect/SPIRV/IR/SPIRVStructureOps.td (+16-7)
- (modified) mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp (+44-28)
- (modified) mlir/test/Dialect/SPIRV/IR/structure-ops.mlir (+2)
``````````diff
diff --git a/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVStructureOps.td b/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVStructureOps.td
index c8cd881f138c5..e1959dbf8767e 100644
--- a/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVStructureOps.td
+++ b/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVStructureOps.td
@@ -28,6 +28,7 @@ include "mlir/Interfaces/SideEffectInterfaces.td"
def SPIRV_AddressOfOp : SPIRV_Op<"mlir.addressof",
[DeclareOpInterfaceMethods<OpAsmOpInterface, ["getAsmResultNames"]>,
+ DeclareOpInterfaceMethods<SymbolUserOpInterface>,
InFunctionScope, Pure]> {
let summary = "Get the address of a global variable.";
@@ -62,6 +63,8 @@ def SPIRV_AddressOfOp : SPIRV_Op<"mlir.addressof",
let builders = [OpBuilder<(ins "spirv::GlobalVariableOp":$var)>];
let assemblyFormat = "$variable attr-dict `:` type($pointer)";
+
+ let hasVerifier = 0;
}
// -----
@@ -290,7 +293,8 @@ def SPIRV_ExecutionModeOp : SPIRV_Op<"ExecutionMode", [InModuleScope]> {
// -----
-def SPIRV_ExecutionModeIdOp : SPIRV_Op<"ExecutionModeId", []> {
+def SPIRV_ExecutionModeIdOp : SPIRV_Op<"ExecutionModeId", [
+ DeclareOpInterfaceMethods<SymbolUserOpInterface>]> {
let summary = [{
Declare an execution mode for an entry point, using <id>s as Extra
Operands.
@@ -465,9 +469,9 @@ def SPIRV_FuncOp : SPIRV_Op<"func", [
// -----
-def SPIRV_GlobalVariableOp
- : SPIRV_Op<"GlobalVariable", [InModuleScope, SymbolName, SymbolVisibility,
- Symbol]> {
+def SPIRV_GlobalVariableOp : SPIRV_Op<"GlobalVariable", [
+ InModuleScope, SymbolName, SymbolVisibility, Symbol,
+ DeclareOpInterfaceMethods<SymbolUserOpInterface>]> {
let summary = [{
Allocate an object in memory at module scope. The object is
referenced using a symbol name.
@@ -659,7 +663,8 @@ def SPIRV_ModuleOp : SPIRV_Op<"module",
// -----
-def SPIRV_ReferenceOfOp : SPIRV_Op<"mlir.referenceof", [Pure]> {
+def SPIRV_ReferenceOfOp : SPIRV_Op<"mlir.referenceof", [
+ DeclareOpInterfaceMethods<SymbolUserOpInterface>, Pure]> {
let summary = "Reference a specialization constant.";
let description = [{
@@ -692,6 +697,8 @@ def SPIRV_ReferenceOfOp : SPIRV_Op<"mlir.referenceof", [Pure]> {
let autogenSerialization = 0;
let assemblyFormat = "$spec_const attr-dict `:` type($reference)";
+
+ let hasVerifier = 0;
}
// -----
@@ -750,7 +757,8 @@ def SPIRV_SpecConstantOp
// -----
def SPIRV_SpecConstantCompositeOp : SPIRV_Op<"SpecConstantComposite", [
- InModuleScope, SymbolName, SymbolVisibility, Symbol]> {
+ InModuleScope, SymbolName, SymbolVisibility, Symbol,
+ DeclareOpInterfaceMethods<SymbolUserOpInterface>]> {
let summary = "Declare a new composite specialization constant.";
let description = [{
@@ -806,7 +814,8 @@ def SPIRV_SpecConstantCompositeOp : SPIRV_Op<"SpecConstantComposite", [
def SPIRV_EXTSpecConstantCompositeReplicateOp
: SPIRV_ExtVendorOp<"SpecConstantCompositeReplicate",
- [InModuleScope, SymbolName, SymbolVisibility, Symbol]> {
+ [InModuleScope, SymbolName, SymbolVisibility, Symbol,
+ DeclareOpInterfaceMethods<SymbolUserOpInterface>]> {
let summary = "Declare a new replicated composite specialization constant op.";
let description = [{
diff --git a/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp b/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
index 35cb37a67f530..47deba50ffbdb 100644
--- a/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
+++ b/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
@@ -358,10 +358,10 @@ void spirv::AddressOfOp::build(OpBuilder &builder, OperationState &state,
build(builder, state, var.getType(), SymbolRefAttr::get(var));
}
-LogicalResult spirv::AddressOfOp::verify() {
- auto varOp = dyn_cast_or_null<spirv::GlobalVariableOp>(
- SymbolTable::lookupNearestSymbolFrom((*this)->getParentOp(),
- getVariableAttr()));
+LogicalResult
+spirv::AddressOfOp::verifySymbolUses(SymbolTableCollection &symbolTable) {
+ auto varOp = symbolTable.lookupNearestSymbolFrom<spirv::GlobalVariableOp>(
+ *this, getVariableAttr());
if (!varOp) {
return emitOpError("expected spirv.GlobalVariable symbol");
}
@@ -1105,17 +1105,19 @@ LogicalResult spirv::ExecutionModeIdOp::verify() {
<< schema.numOperands << " value operand(s), got "
<< getValues().size();
- for (Attribute value : getValues()) {
- auto valueSymbol = dyn_cast<FlatSymbolRefAttr>(value);
- if (!valueSymbol)
- return emitOpError("expected value operands to be symbol reference");
- Operation *valueOp = SymbolTable::lookupNearestSymbolFrom(
- (*this)->getParentOp(), valueSymbol);
- if (!valueOp)
+ if (!llvm::all_of(getValues(), llvm::IsaPred<FlatSymbolRefAttr>))
+ return emitOpError("expected value operands to be symbol reference");
+
+ return success();
+}
+
+LogicalResult
+spirv::ExecutionModeIdOp::verifySymbolUses(SymbolTableCollection &symbolTable) {
+ for (auto valueSymbol : getValues().getAsRange<FlatSymbolRefAttr>()) {
+ if (!symbolTable.lookupNearestSymbolFrom(*this, valueSymbol))
return emitOpError("cannot find symbol referenced by value operand: ")
<< valueSymbol.getValue();
}
-
return success();
}
@@ -1511,9 +1513,14 @@ LogicalResult spirv::GlobalVariableOp::verify() {
}
}
+ Type pointeeType = cast<spirv::PointerType>(getType()).getPointeeType();
+ return verifyPhysicalStorageBufferDecorations(getOperation(), pointeeType);
+}
+
+LogicalResult
+spirv::GlobalVariableOp::verifySymbolUses(SymbolTableCollection &symbolTable) {
if (FlatSymbolRefAttr init = getInitializerAttr()) {
- Operation *initOp = SymbolTable::lookupNearestSymbolFrom(
- (*this)->getParentOp(), init.getAttr());
+ Operation *initOp = symbolTable.lookupNearestSymbolFrom(*this, init);
// TODO: Currently only variable initialization with specialization
// constants is supported. There could be normal constants in the module
// scope as well.
@@ -1532,11 +1539,6 @@ LogicalResult spirv::GlobalVariableOp::verify() {
}
}
- Type pointeeType = cast<spirv::PointerType>(getType()).getPointeeType();
- if (failed(
- verifyPhysicalStorageBufferDecorations(getOperation(), pointeeType)))
- return failure();
-
return success();
}
@@ -1847,9 +1849,10 @@ LogicalResult spirv::ModuleOp::verifyRegions() {
// spirv.mlir.referenceof
//===----------------------------------------------------------------------===//
-LogicalResult spirv::ReferenceOfOp::verify() {
- auto *specConstSym = SymbolTable::lookupNearestSymbolFrom(
- (*this)->getParentOp(), getSpecConstAttr());
+LogicalResult
+spirv::ReferenceOfOp::verifySymbolUses(SymbolTableCollection &symbolTable) {
+ auto *specConstSym =
+ symbolTable.lookupNearestSymbolFrom(*this, getSpecConstAttr());
Type constType;
auto specConstOp = dyn_cast_or_null<spirv::SpecConstantOp>(specConstSym);
@@ -2036,11 +2039,19 @@ LogicalResult spirv::SpecConstantCompositeOp::verify() {
<< cType.getNumElements() << ", but provided "
<< constituents.size();
+ return success();
+}
+
+LogicalResult spirv::SpecConstantCompositeOp::verifySymbolUses(
+ SymbolTableCollection &symbolTable) {
+ auto cType = cast<spirv::CompositeType>(getType());
+ auto constituents = this->getConstituents().getValue();
+
for (auto index : llvm::seq<uint32_t>(0, constituents.size())) {
auto constituent = cast<FlatSymbolRefAttr>(constituents[index]);
- Operation *constituentOp = SymbolTable::lookupNearestSymbolFrom(
- (*this)->getParentOp(), constituent.getAttr());
+ Operation *constituentOp =
+ symbolTable.lookupNearestSymbolFrom(*this, constituent);
if (!constituentOp)
return emitError("unknown constituent symbol ") << constituent.getAttr();
@@ -2110,13 +2121,18 @@ void spirv::EXTSpecConstantCompositeReplicateOp::print(OpAsmPrinter &printer) {
}
LogicalResult spirv::EXTSpecConstantCompositeReplicateOp::verify() {
- auto compositeType = dyn_cast<spirv::CompositeType>(getType());
- if (!compositeType)
+ if (!isa<spirv::CompositeType>(getType()))
return emitError("result type must be a composite type, but provided ")
<< getType();
- Operation *constituentOp = SymbolTable::lookupNearestSymbolFrom(
- (*this)->getParentOp(), this->getConstituent());
+ return success();
+}
+
+LogicalResult spirv::EXTSpecConstantCompositeReplicateOp::verifySymbolUses(
+ SymbolTableCollection &symbolTable) {
+ auto compositeType = cast<spirv::CompositeType>(getType());
+ Operation *constituentOp =
+ symbolTable.lookupNearestSymbolFrom(*this, getConstituent());
if (!constituentOp)
return emitError(
"splat spec constant reference defining constituent not found");
diff --git a/mlir/test/Dialect/SPIRV/IR/structure-ops.mlir b/mlir/test/Dialect/SPIRV/IR/structure-ops.mlir
index 4acaa417501ac..996755ab7783e 100644
--- a/mlir/test/Dialect/SPIRV/IR/structure-ops.mlir
+++ b/mlir/test/Dialect/SPIRV/IR/structure-ops.mlir
@@ -33,6 +33,7 @@ spirv.module Logical GLSL450 {
spirv.func @foo() -> () "None" {
// expected-error @+1 {{expected spirv.GlobalVariable symbol}}
%0 = spirv.mlir.addressof @var2 : !spirv.ptr<!spirv.struct<(f32, !spirv.array<4xf32>)>, Input>
+ spirv.Return
}
}
@@ -43,6 +44,7 @@ spirv.module Logical GLSL450 {
spirv.func @foo() -> () "None" {
// expected-error @+1 {{result type mismatch with the referenced global variable's type}}
%0 = spirv.mlir.addressof @var1 : !spirv.ptr<f32, Input>
+ spirv.Return
}
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/235042
More information about the Mlir-commits
mailing list