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

[Nicolas Vasilache via Phabricator via llvm-commits]

nicolasvasilache marked 2 inline comments as done.
nicolasvasilache added inline comments.


================
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,
----------------
nicolasvasilache wrote:
> 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.
OTOH it's true the multiple possibilities make it more confusing and the most important, explicitly spelled out,  use case (i.e. 1 capture) works with MutableArrayRef.
I just went ahead and updated everything with MutableArrayRef.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78691