Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor API fixes for TTNN encoding ettribute #1390

Merged
merged 5 commits into from
Nov 27, 2024
Merged

Conversation

mtopalovicTT
Copy link
Contributor

This PR adds couple convenience methods to TTNN tensor encoding attribute and also removes redundant utils functions.

Renaming/Adding some new functions...

  • getDataType to get scalar data type:
    • memref<2x2x!tt.tile<32x32xf32>> returns float data type
    • memref<128x128xi32> returns int data type
  • getElementType to get type from memref:
    • memref<2x2x!tt.tile<32x32xf32>> returns TileType
    • memref<128x128xi32> returns IntegerType
  • getLayout - gets layout of encoding i.e Tile/RowMajor
  • getShardShape:
    • memref<2x2x!tt.tile<32x32xf32>> returns (2, 2)
    • memref<128x128xi32> returns (128, 128)
  • getScalarShardShape:
    • memref<2x2x!tt.tile<32x32xf32>> returns (64, 64)
    • memref<128x128xi32> returns (128, 128)

Comment on lines 112 to 114
Here is how tensor will look like after layout tensor<32x32x64xf32, #ttnn.ttnn_layout<linear, grid, memref, mem_layout>>
Lets break down what each parameter means:
- affine_map: An affine map that defines how the logical tensor dimensions map to physical space.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

linear vs affine_map

@@ -190,25 +190,13 @@ ::mlir::LogicalResult mlir::tt::ttnn::EmptyOp::verify() {

// DataType and Layout
//
mlir::MemRefType memref = layoutAttr.getMemref();
Type elementType = memref.getElementType();
// Type elementType = layoutAttr.getElementType();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stray comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep.. Tnx for noticing

Comment on lines 39 to 43
if (isTiled()) {
return Layout::Tile;
}

return Layout::RowMajor;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider packing into a one-liner.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

// Example: memref<128x128xf32> -> { 128, 128 }
// Example: memref<2x3!tt.tile<32x32xf32>> -> { 64, 96 }
//
// /return The scalar shape of the shard.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with this /return, is it a docs thing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep yep

Copy link
Contributor Author

@mtopalovicTT mtopalovicTT Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I now see that I ill formed comments. Doxygen requires /// for comments in order to generate documentation.
Also return param should be prefixed with \ not /.

@mtopalovicTT mtopalovicTT enabled auto-merge (squash) November 27, 2024 09:31
@mtopalovicTT mtopalovicTT merged commit d22057f into main Nov 27, 2024
18 checks passed
@mtopalovicTT mtopalovicTT deleted the milant/minor_cleanup branch December 31, 2024 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants