[Mlir-commits] [mlir] [mlir][Interfaces] Add `ExecutionProgressOpInterface` + folding pattern (PR #179039)

[Mehdi Amini]

================
@@ -0,0 +1,23 @@
+//===- ExecutionProgressOpInterface.cpp -- Execution Progress Interface ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "mlir/Interfaces/ExecutionProgressOpInterface.h"
+
+using namespace mlir;
+
+namespace mlir {
+#include "mlir/Interfaces/ExecutionProgressOpInterface.cpp.inc"
+} // namespace mlir
+
+bool mlir::mustProgress(Operation *op) {
+  auto executionProgressOpInterface =
+      dyn_cast<ExecutionProgressOpInterface>(op);
+  if (!executionProgressOpInterface)
+    return false;
----------------
joker-eph wrote:

> I changed this based on @zero9178's comment above: If we don't know whether an op must progress or not, it's better to give a conservative answer. A return value of "false" doesn't give us any information: the op may progress or it may not.


Right, I understand the rational, but I don't agree with the conclusion here in this case: and we discussed about it so you should have checked with me before changing this / merging this PR.
This makes the common case overly expensive, and I'm not convinced this is the right tradeoff.

> Correct. I'd like to highlight that this does not imply that the op does not progress. It just means that we do not know.

Sure, so we need to be conservative, which I don't believe is right. And more importantly, we'll have to pay some extra cost (extra call) for the majority of the ops which are progressing.


https://github.com/llvm/llvm-project/pull/179039