Reshape only in row major layout #687
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TTNN supports two kinds of reshapes - row and tile. row reshape works as expected as long as we don't change last dim. On the other hand tile reshape requires that output `W` and `H` dims are tile aligned. For example this `reshape` in tile layout will fail: 1*12*32 -> 12*32 Error that we get is `Unable to reshape a tensor in TILE_LAYOUT to non-tile height and width! Please convert the tensor to ROW_MAJOR_LAYOUT first.` In addition when we managed to do a `reshape` on tile tensor we found that output tensor did not look the way we expected it to look. For example: If we have input row major tensor 4x4 which contains numbers from 0 .. 15 and we reshape it to 2x8 we will get [[0,1 .. 6,7], [8,9 .. 14,15]]. The output is correct. If we take the same input tensor and convert it to tile layout and call reshape to Shape((2, 8), (32, 32)) and convert output tensor to row_major we will get tensor which looks like this: [[0,1,2,3,0,0,0,0],[4,5,6,7,0,0,0,0]].This is different then row major reshape. In order to unblock forge for `reshape` op we will convert all input tensors to row before calling reshape, and then we will convert output tensor back to original layout. Once we have understanding how reshape works for tile we will remove this and implement proper fix.
mtopalovicTT
requested review from
jnie-TT,
kmabeeTT,
AleksKnezevic and
nsmithtt
as code owners
mtopalovicTT
changed the title
nsmithtt
reviewed
Sep 13, 2024
| %1 = "ttir.reshape"(%arg0, %0) <{shape = [4: i32, 32: i32, 2: i32, 32: i32] , operand_constraints = [#any_device_tile, #any_device_tile]}> : (tensor<4x2x32x32xbf16>, tensor<4x32x2x32xbf16>) -> tensor<4x32x2x32xbf16> | ||
| return %1 : tensor<4x32x2x32xbf16> | ||
| } | ||
| } |
There was a problem hiding this comment.
Should these 2 tests belong in Silicon subdir? Otherwise they will not be exercising the runtime in CI
There was a problem hiding this comment.
If thats the case we will have to move bunch of tests from dialect dir to silicon, since we are not testing bunch of ops.
sdjordjevicTT
approved these changes
Sep 16, 2024
mtopalovicTT
pushed a commit
that referenced
this pull request
Sep 16, 2024
This reverts commit 8152560.
mtopalovicTT
added a commit
that referenced
this pull request
Sep 16, 2024
uazizTT
pushed a commit
that referenced
this pull request
Sep 16, 2024
* Convert to `row_major` layout before calling `reshape` TTNN supports two kinds of reshapes - row and tile. row reshape works as expected as long as we don't change last dim. On the other hand tile reshape requires that output `W` and `H` dims are tile aligned. For example this `reshape` in tile layout will fail: 1*12*32 -> 12*32 Error that we get is `Unable to reshape a tensor in TILE_LAYOUT to non-tile height and width! Please convert the tensor to ROW_MAJOR_LAYOUT first.` In addition when we managed to do a `reshape` on tile tensor we found that output tensor did not look the way we expected it to look. For example: If we have input row major tensor 4x4 which contains numbers from 0 .. 15 and we reshape it to 2x8 we will get [[0,1 .. 6,7], [8,9 .. 14,15]]. The output is correct. If we take the same input tensor and convert it to tile layout and call reshape to Shape((2, 8), (32, 32)) and convert output tensor to row_major we will get tensor which looks like this: [[0,1,2,3,0,0,0,0],[4,5,6,7,0,0,0,0]].This is different then row major reshape. In order to unblock forge for `reshape` op we will convert all input tensors to row before calling reshape, and then we will convert output tensor back to original layout. Once we have understanding how reshape works for tile we will remove this and implement proper fix. * Adding comment * Moving test to Silicon. Adding negative test for dialect.
uazizTT
pushed a commit
that referenced
this pull request
Sep 16, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reshape doesn't work for most output shapes in tile layout. As temporary workaround (until we figure out how to do a reshape in tile properly) and to unblock forge we will convert all input tensors to row before calling reshape, and then we will convert output tensor back to original layout.
#686