[mlir] add missing pessimistic setting

[qingyunqu]

Summary

This is updated on https://reviews.llvm.org/D127139, to mark op with region as pessimistic.

[jpienaar]

Matches the comment and what was there before (https://github.com/llvm/llvm-project/blob/2c384c37727660f11f63fda461210d1a6f5d2afe/mlir/lib/Transforms/SCCP.cpp#L84). Seems SCCP has some missing test coverage.

Do you have a test case that triggers for this one?

[qingyunqu]

In D131480#3709709, @jpienaar wrote:

Matches the comment and what was there before (https://github.com/llvm/llvm-project/blob/2c384c37727660f11f63fda461210d1a6f5d2afe/mlir/lib/Transforms/SCCP.cpp#L84). Seems SCCP has some missing test coverage.

Do you have a test case that triggers for this one?

I trigger this when working on mhlo.reduce op. But I think I could create a test like https://reviews.llvm.org/D130453

[jpienaar]

In D131480#3711409, @qingyunqu wrote:

In D131480#3709709, @jpienaar wrote:

Matches the comment and what was there before (https://github.com/llvm/llvm-project/blob/2c384c37727660f11f63fda461210d1a6f5d2afe/mlir/lib/Transforms/SCCP.cpp#L84). Seems SCCP has some missing test coverage.

Do you have a test case that triggers for this one?

I trigger this when working on mhlo.reduce op. But I think I could create a test like https://reviews.llvm.org/D130453

Thanks that would be great.

[qingyunqu]

In D131480#3712424, @jpienaar wrote:

In D131480#3711409, @qingyunqu wrote:

In D131480#3709709, @jpienaar wrote:

Matches the comment and what was there before (https://github.com/llvm/llvm-project/blob/2c384c37727660f11f63fda461210d1a6f5d2afe/mlir/lib/Transforms/SCCP.cpp#L84). Seems SCCP has some missing test coverage.

Do you have a test case that triggers for this one?

I trigger this when working on mhlo.reduce op. But I think I could create a test like https://reviews.llvm.org/D130453

Thanks that would be great.

Sorry for delay, I have added the test now.

[jpienaar]

Thanks, looks good.

mlir/lib/Analysis/DataFlow/ConstantPropagationAnalysis.cpp

void SparseConstantPropagation::visitOperation(
     Operation *op, ArrayRef<const Lattice<ConstantValue> *> operands,
     ArrayRef<Lattice<ConstantValue> *> results) {
   LLVM_DEBUG(llvm::dbgs() << "SCP: Visiting operation: " << *op << "\n");
 
   // Don't try to simulate the results of a region operation as we can't
   // guarantee that folding will be out-of-place. We don't allow in-place
   // folds as the desire here is for simulated execution, and not general
   // folding.
-  if (op->getNumRegions())
+  if (op->getNumRegions()) {
+    markAllPessimisticFixpoint(results);
     return;
+  }
 
   SmallVector<Attribute, 8> constantOperands;
   constantOperands.reserve(op->getNumOperands());
   for (auto *operandLattice : operands)
     constantOperands.push_back(operandLattice->getValue().getConstantValue());
 
   // Save the original operands and attributes just in case the operation
   // folds in-place. The constant passed in may not correspond to the real

mlir/test/Transforms/sccp.mlir

 ^bb2(%arg1 : i32, %arg2 : i32):
   // CHECK: ^bb2(%[[ARG:.*]]: i32, %{{.*}}: i32):
   // CHECK: return %[[ARG]], %[[ONE]] : i32, i32
 
   return %arg1, %arg2 : i32, i32
 }
 
 // CHECK-LABEL: inplace_fold
-func.func @inplace_fold(%arg: i1) -> (i32) {
+func.func @inplace_fold() -> (i32) {
   %0 = "test.op_in_place_fold_success"() : () -> i1
   %1 = arith.constant 5 : i32
   cf.cond_br %0, ^a, ^b
 
 ^a:
   // CHECK-NOT: addi
   %3 = arith.addi %1, %1 : i32
   return %3 : i32
 
 ^b:
   return %1 : i32
 }
+
+// CHECK-LABEL: op_with_region
+func.func @op_with_region() -> (i32) {
+  %0 = "test.op_with_region"() ({}) : () -> i1
+  %1 = arith.constant 5 : i32
+  cf.cond_br %0, ^a, ^b
+
+^a:
+  // CHECK-NOT: addi
+  %3 = arith.addi %1, %1 : i32
+  return %3 : i32
+
+^b:
+  return %1 : i32
+}