[PATCH] D78691: [mlir][EDSC] Retire ValueHandle

[Nicolas Vasilache via Phabricator via llvm-commits]

nicolasvasilache marked 14 inline comments as done.
nicolasvasilache added a comment.

Thanks for the review!



================
Comment at: mlir/include/mlir/Dialect/StandardOps/EDSC/Intrinsics.h:96
+std_cond_br(Value cond, BlockHandle *trueBranch, ArrayRef<Type> trueTypes,
+            ArrayRef<Value *> trueCaptures, ArrayRef<Value> trueOperands,
+            BlockHandle *falseBranch, ArrayRef<Type> falseTypes,
----------------
ftynse wrote:
> MutableArrayRef instead of ArrayRef<Value *> ?  Okay for a separate commit, but you seem to be doing this already in loop builders.
re `MutableArrayRef<T>` vs `ArrayRef<T*>`, both have issues: `MutableArrayRef` does not work with initializer lists but can be constructed from 2 pointers (begin, end) which leads to fun behavior. OTOH `ArrayRef` + vectors is clunky (i.e. the retired `makeVectorHandlePtrs`). For now I added the minimum necessary, we can reevaluate in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78691