Skip to content

Conversation

@krzysz00
Copy link
Contributor

When adding a predicated field to non-attribute properties / implemneting PropConstraint, a call to genPropertyVerifiers() wasn't added to the generation sequence for [Op]GenericAdaptor::verify. This commit fixes the issue.

When adding a predicated field to non-attribute properties /
implemneting PropConstraint, a call to genPropertyVerifiers() wasn't
added to the generation sequence for [Op]GenericAdaptor::verify. This
commit fixes the issue.
@krzysz00 krzysz00 requested a review from joker-eph July 28, 2025 05:35
@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir labels Jul 28, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 28, 2025

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-core

Author: Krzysztof Drewniak (krzysz00)

Changes

When adding a predicated field to non-attribute properties / implemneting PropConstraint, a call to genPropertyVerifiers() wasn't added to the generation sequence for [Op]GenericAdaptor::verify. This commit fixes the issue.


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

1 Files Affected:

  • (modified) mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp (+2-1)
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index f35cfa6826388..8ea4eb7b3eeca 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -1127,7 +1127,7 @@ static void genPropertyVerifier(
     body << formatv(fetchProperty, varName, getterName,
                     prop.prop.getInterfaceType());
     auto uniquedFn = staticVerifierEmitter.getPropConstraintFn(prop.prop);
-    if (uniquedFn.has_value())
+    if (uniquedFn.has_value() && emitHelper.isEmittingForOp())
       body << formatv(verifyPropertyUniqued, *uniquedFn, varName, prop.name);
     else
       body << formatv(
@@ -4764,6 +4764,7 @@ void OpOperandAdaptorEmitter::addVerification() {
 
   FmtContext verifyCtx;
   populateSubstitutions(emitHelper, verifyCtx);
+  genPropertyVerifier(emitHelper, verifyCtx, body, staticVerifierEmitter);
   genAttributeVerifier(emitHelper, verifyCtx, body, staticVerifierEmitter,
                        useProperties);
 

Copy link
Member

@zero9178 zero9178 left a comment

Choose a reason for hiding this comment

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

Can we add a test exercising the new adapter verifier parts or at least checking they're in the Tablegen output?

Copy link
Collaborator

@joker-eph joker-eph left a comment

Choose a reason for hiding this comment

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

Can you add a test?

@krzysz00 krzysz00 merged commit bd04138 into main Jul 31, 2025
9 checks passed
@krzysz00 krzysz00 deleted the users/krzysz00/missing-verifiers-in-adaptor branch July 31, 2025 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mlir:core MLIR Core Infrastructure mlir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants