[PATCH] D106422: Implement recursive support into OperationEquivalence::isEquivalentTo()

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini marked 7 inline comments as done.
mehdi_amini added inline comments.


================
Comment at: mlir/lib/Transforms/CSE.cpp:32-33
+    return OperationEquivalence::computeHash(
+        const_cast<Operation *>(opC), [](Value v) { return hash_value(v); },
+        [](Value v) { return llvm::hash_code{}; },
+        OperationEquivalence::IgnoreLocations);
----------------
rriddle wrote:
> Can you add param comments?
> 
> Also, is it possible to just pass hash_value directly for the operand hash? Or do you need to wrap it?
I need to wrap (unless you see another way) because:
```
ERROR: mlir/lib/Transforms/CSE.cpp:33:26 no viable conversion from '<overloaded function type>' to 'function_ref<llvm::hash_code (mlir::Value)>'
        /*hashOperands=*/hash_value,
                         ^~~~~~~~~~
llvm/include/llvm/ADT/STLExtras.h:171:7 candidate constructor (the implicit copy constructor) not viable: no overload of 'hash_value' matching 'const llvm::function_ref<llvm::hash_code (mlir::Value)> &' for 1st argument
class function_ref<Ret(Params...)> {
      ^
llvm/include/llvm/ADT/STLExtras.h:171:7 candidate constructor (the implicit move constructor) not viable: no overload of 'hash_value' matching 'llvm::function_ref<llvm::hash_code (mlir::Value)> &&' for 1st argument
llvm/include/llvm/ADT/STLExtras.h:183:3 candidate constructor not viable: no overload of 'hash_value' matching 'std::nullptr_t' (aka 'nullptr_t') for 1st argument
  function_ref(std::nullptr_t) {}
  ^
llvm/include/llvm/ADT/STLExtras.h:186:3 candidate template ignored: couldn't infer template argument 'Callable'
  function_ref(
  ^
mlir/include/mlir/IR/OperationSupport.h:915:44 passing argument to parameter 'hashOperands' here
      function_ref<llvm::hash_code(Value)> hashOperands =
                                           ^
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106422/new/

https://reviews.llvm.org/D106422