-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[mlir][python] Fix ir.Value type to not break other types #167930
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
Conversation
|
@llvm/pr-subscribers-mlir-core @llvm/pr-subscribers-mlir Author: Nirvedh Meshram (nirvedhmeshram) ChangesChange in #166148 caused breaks for some other types. This PR tries to make those changes not affect the other types Full diff: https://github.com/llvm/llvm-project/pull/167930.diff 1 Files Affected:
diff --git a/mlir/tools/mlir-tblgen/OpPythonBindingGen.cpp b/mlir/tools/mlir-tblgen/OpPythonBindingGen.cpp
index f01563fd49d17..d0a25149cfe2a 100644
--- a/mlir/tools/mlir-tblgen/OpPythonBindingGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpPythonBindingGen.cpp
@@ -437,7 +437,7 @@ static void emitElementAccessors(
type = std::strcmp(kind, "operand") == 0 ? "_ods_ir.Value"
: "_ods_ir.OpResult";
}
- if (std::strcmp(kind, "operand") == 0) {
+ if (std::strcmp(type.c_str(), "_ods_ir.Value") == 0) {
StringRef pythonType = getPythonType(element.constraint.getCppType());
if (!pythonType.empty())
type += "[" + pythonType.str() + "]";
@@ -473,7 +473,7 @@ static void emitElementAccessors(
if (!element.isVariableLength() || element.isOptional()) {
type = std::strcmp(kind, "operand") == 0 ? "_ods_ir.Value"
: "_ods_ir.OpResult";
- if (std::strcmp(kind, "operand") == 0) {
+ if (std::strcmp(type.c_str(), "_ods_ir.Value") == 0) {
StringRef pythonType = getPythonType(element.constraint.getCppType());
if (!pythonType.empty())
type += "[" + pythonType.str() + "]";
|
Signed-off-by: Nirvedh Meshram <nirvedh@gmail.com>
|
@nirvedhmeshram , looking a little bit closer, both |
|
@amd-eochoalo please let me know if my new change would be the right way to handle that. |
|
@nirvedhmeshram it is sufficiently good :) Thanks! |
makslevental
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Change in llvm#166148 caused breaks for some other types. Specifically this error was seen in a downstream project ``` _ods_ir.OpOperandList[_ods_ir.IntegerType]: TypeError: type 'iree.compiler._mlir_libs._mlir.ir.OpOperandList' is not subscriptable ``` This PR tries to make those changes not affect the other types --------- Signed-off-by: Nirvedh Meshram <nirvedh@gmail.com>
Change in #166148 caused breaks for some other types.
Specifically this error was seen in a downstream project
This PR tries to make those changes not affect the other types