[PATCH] D78892: [mlir] Add a new MutableOperandRange class for adding/remove operands

[River Riddle via Phabricator via llvm-commits]

rriddle added inline comments.


================
Comment at: mlir/include/mlir/IR/Operation.h:214
+  /// Insert the given operands into the operand list at the given 'index'.
+  void insertOperands(unsigned index, ValueRange operands);
+
----------------
jpienaar wrote:
> How does this affects ops with AttrSizedOperandSegments? E.g., if someone were to call this function manually on an op that implements AttrSizedOperandSegments and inserts an operand in the middle of a segment, what happens?
The methods on Operation don't know about the presence of AttrSizedOperandSegments and shouldn't have to IMO. It is much more efficient to let the users that know about this handling(generally ODS and the derived op) take care of it themselves. Otherwise, the cost of checking for the presence of that becomes costly. I have generally been trying to avoid creeping the presence of traits into Operation unless there is a clear win, either in memory size or performance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78892