[Mlir-commits] [mlir] [mlir] Change `tensor.extract/insert` to take static/dynamic indices. (PR #104488)

llvmlistbot at llvm.org llvmlistbot at llvm.org
Mon Aug 19 18:54:00 PDT 2024


MaheshRavishankar wrote:

> >  It's opt-in based on the op.
> 
> I don't quite get what you mean by this?

I mean it only matters to the op that wants to use this paradigm. So in that sense it is opt-in. 

> 
> >  So what is the complexity you are referring to?
> 
> The C++ API provides accessors for either Value or Attribute which describe the same thing and are both optional (one of them must be present, in an exclusionary way).

Not sure that is true. As I understand it the attribute is always required. The Values are needed only if the attribute has a sentinel value saying the dimension is dynamic.

> MLIR core infra is just not setup to support this use case very well (we could have improved it to do this, but it isn't a pattern that has been widely accepted and so it hasn't happened). For example you can use getOperands() to get a `ValueRange`, and ODS can generate accessors for `ValueRange` for named operands, but this "idiom" is not correctly abstracted anywhere in the same way and the complexity is pushed on every API users.

That is true. Most ops that use this use custom methods to get mixed static/dynamic values.  This PR IMO is starting to build support for this paradigm and make it's usage more uniform


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


More information about the Mlir-commits mailing list