[spec] Add dtype and layout arguments to ttl.block.fill#611
[spec] Add dtype and layout arguments to ttl.block.fill#611phizalev-TT wants to merge 6 commits into
dtype and layout arguments to ttl.block.fill#611Conversation
| | Function | Description | | ||
| | :---- | :---- | | ||
| | `ttl.block.fill(value: float, shape: ttl.Shape) -> ttl.BlockExpr` | Fill a block of specified `shape` with specified `value`. | | ||
| | `ttl.block.fill(value: float, dtype: ttnn.DataType, shape: ttl.Shape) -> ttl.BlockExpr` | Fill a block of specified `shape` with specified `value` of specified `dtype`. | |
There was a problem hiding this comment.
Since this is a change in the signature of fill and it is not an optional argument, I expect all instances of fill will have to be changed throughout this document?
There was a problem hiding this comment.
Yes, that's true -- and not just the document, but examples, too; I can do that in the still-open #599 -- or you @FConstantinos feel free to commit sim changes there directly (if we agree to merge the spec change first before 599 lands).
There was a problem hiding this comment.
Actually, 599 is already too big, so ignore the above. We can just do the dtype addition separately in subsequent PRs. I'll have to change the compiler's fill implementation anyway since currently dtype is optional (and last).
|
This feels like a language gap. |
I think the problem is that there is no concise way to create a correctly-typed constant host scalar in Python. Users would have to call something like |
|
Do we want to add a version row? |
|
re I also understand that we never deal with tensors of integers, is that accurate? |
|
Also, should we consider a default for dtype? (e.g float32) or do we need to make this non-optional? |
| | Function | Description | | ||
| | :---- | :---- | | ||
| | `ttl.block.fill(value: float, shape: ttl.Shape) -> ttl.BlockExpr` | Fill a block of specified `shape` with specified `value`. | | ||
| | `ttl.block.fill(value: float, dtype: ttnn.DataType, shape: ttl.Shape) -> ttl.BlockExpr` | Fill a block of specified `shape` with specified `value` of specified `dtype`. | |
There was a problem hiding this comment.
is ttnn.DataType defined anywhere?
There was a problem hiding this comment.
I don't think the spec needs to define ttnn entities, that's up to the ttnn docs
There was a problem hiding this comment.
We rely on ttnn.Tensor in the same way. It is defined by TT=NN.
| | Function | Description | | ||
| | :---- | :---- | | ||
| | `ttl.block.fill(value: float, shape: ttl.Shape) -> ttl.BlockExpr` | Fill a block of specified `shape` with specified `value`. | | ||
| | `ttl.block.fill(value: float, dtype: ttnn.DataType, shape: ttl.Shape) -> ttl.BlockExpr` | Fill a block of specified `shape` with specified `value` of specified `dtype`. | |
There was a problem hiding this comment.
I guess an important question is, how do we know the layout of the fill (TILE_LAYOUT vs ROW_MAJOR_LAYOUT)? Perhaps we need to add more arguments that specify this? Or perhaps say that fill always produces a tiled block?
There was a problem hiding this comment.
The sim supports multiple layouts, so this does seem necessary, perhaps make it an optional arg with a default TILE_LAYOUT?
I also think optional with some reasonable default is good. I think bf16 is WAY more common though (and f32 has quite a few bugs in the llks, hardware), so I think having it be the default doesn't actually help the compiler much since that means we have to explicitly have |
We can create integer by using corresponding dtype, which will truncate float constant. Same goes for any other type. |
This is minor change and with default |
dtype argument to ttl.block.filldtype and layout arguments to ttl.block.fill
No description provided.