[Mlir-commits] [mlir] b9c353f - [mlir] Use PyValue instead of PyOpResult in Python operand container

[Alex Zinenko]

Author: Alex Zinenko
Date: 2020-11-06T19:02:35+01:00
New Revision: b9c353fabb3276c9ec1c202f28559d37c8092739

URL: https://github.com/llvm/llvm-project/commit/b9c353fabb3276c9ec1c202f28559d37c8092739
DIFF: https://github.com/llvm/llvm-project/commit/b9c353fabb3276c9ec1c202f28559d37c8092739.diff

LOG: [mlir] Use PyValue instead of PyOpResult in Python operand container

The PyOpOperands container was erroneously constructing objects for
individual operands as PyOpResult. Operands in fact are just values,
which may or may not be results of another operation. The code would
eventually crash if the operand was a block argument. Add a test that
exercises the behavior that previously led to crashes.

Reviewed By: stellaraccident

Differential Revision: https://reviews.llvm.org/D90917

Added: 
    

Modified: 
    mlir/lib/Bindings/Python/IRModules.cpp
    mlir/test/Bindings/Python/ir_operation.py

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Bindings/Python/IRModules.cpp b/mlir/lib/Bindings/Python/IRModules.cpp
index 5a3e96661106..cf71cc3eb92e 100644
--- a/mlir/lib/Bindings/Python/IRModules.cpp
+++ b/mlir/lib/Bindings/Python/IRModules.cpp
@@ -1228,13 +1228,12 @@ class PyOpOperandList {
   }
 
   /// Returns `index`-th element in the result list.
-  PyOpResult dunderGetItem(intptr_t index) {
+  PyValue dunderGetItem(intptr_t index) {
     if (index < 0 || index >= dunderLen()) {
       throw SetPyError(PyExc_IndexError,
                        "attempt to access out of bounds region");
     }
-    PyValue value(operation, mlirOperationGetOperand(operation->get(), index));
-    return PyOpResult(value);
+    return PyValue(operation, mlirOperationGetOperand(operation->get(), index));
   }
 
   /// Defines a Python class in the bindings.

diff  --git a/mlir/test/Bindings/Python/ir_operation.py b/mlir/test/Bindings/Python/ir_operation.py
index d5c5b3f121de..0ce7ceea0a5c 100644
--- a/mlir/test/Bindings/Python/ir_operation.py
+++ b/mlir/test/Bindings/Python/ir_operation.py
@@ -132,6 +132,29 @@ def testBlockArgumentList():
 run(testBlockArgumentList)
 
 
+# CHECK-LABEL: TEST: testOperationOperands
+def testOperationOperands():
+  with Context() as ctx:
+    ctx.allow_unregistered_dialects = True
+    module = Module.parse(r"""
+      func @f1(%arg0: i32) {
+        %0 = "test.producer"() : () -> i64
+        "test.consumer"(%arg0, %0) : (i32, i64) -> ()
+        return
+      }""")
+    func = module.body.operations[0]
+    entry_block = func.regions[0].blocks[0]
+    consumer = entry_block.operations[1]
+    assert len(consumer.operands) == 2
+    # CHECK: Operand 0, type i32
+    # CHECK: Operand 1, type i64
+    for i, operand in enumerate(consumer.operands):
+      print(f"Operand {i}, type {operand.type}")
+
+
+run(testOperationOperands)
+
+
 # CHECK-LABEL: TEST: testDetachedOperation
 def testDetachedOperation():
   ctx = Context()