forked from llvm/llvm-project
-
Notifications
You must be signed in to change notification settings - Fork 0
Dialect assembly format - make offsets optional for create_nd_tdesc #1
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
Closed
Closed
Changes from 5 commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
b9a6d98
init code
Jianhui-Li 2465050
add tests
Jianhui-Li 1077871
git-clang-format
Jianhui-Li 42baa22
add more tests
Jianhui-Li 204d347
git-clang-format
Jianhui-Li 2793c81
add ui64 case support
Jianhui-Li f23ea03
modify ui64 test
Jianhui-Li 0bb958b
Merge branch 'main' into dialect-assembly-format
Jianhui-Li 6793689
remove unnecessary comments
Jianhui-Li 4a96c71
fix VectorToXeGPU tests
Jianhui-Li 689a8a5
tweak default offset value
Jianhui-Li 02d3795
git-clang-format
Jianhui-Li 01718f4
add builders
Jianhui-Li 5ef6ca9
git-clang-format
Jianhui-Li 26a222d
Merge branch 'main' into dialect-assembly-format
Jianhui-Li 882313f
simplify custom parser
Jianhui-Li 456534a
add comma before shape and strides
Jianhui-Li b6f016e
tie the offsets rank to input tensor shape instead of tdesc
Jianhui-Li cd518d2
git-clang-format
Jianhui-Li 546a3f7
addverifier for invalid cases
Jianhui-Li 7846955
git-clang-format
Jianhui-Li ded9552
add comments
Jianhui-Li 97b6e39
simplify custom print
Jianhui-Li ed1d48e
git-clang-format
Jianhui-Li b3edff6
Merge branch 'main' into dialect-assembly-format
Jianhui-Li d3e935b
use simpler interface for DenseI64ArrayAttr
Jianhui-Li 205fea7
address feedback
Jianhui-Li File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -110,23 +110,36 @@ def XeGPU_CreateNdDescOp: XeGPU_Op<"create_nd_tdesc", [Pure, ViewLikeOpInterface | |
| Variadic<Index>: $offsets, | ||
| Variadic<Index>: $shape, | ||
| Variadic<Index>: $strides, | ||
| DenseI64ArrayAttr: $const_offsets, | ||
| OptionalAttr<DenseI64ArrayAttr>: $const_offsets, | ||
| OptionalAttr<DenseI64ArrayAttr>: $const_shape, | ||
| OptionalAttr<DenseI64ArrayAttr>: $const_strides | ||
| ); | ||
| let results = (outs XeGPU_TensorDesc: $TensorDesc); | ||
|
|
||
| let assemblyFormat = [{ | ||
| $source `` | ||
| custom<DynamicIndexList>($offsets, $const_offsets) | ||
| (`,` custom<DynamicIndexList>($shape, $const_shape)^ | ||
| `,` custom<DynamicIndexList>($strides, $const_strides))? | ||
| attr-dict `:` type($source) `->` qualified(type($TensorDesc)) | ||
| }]; | ||
|
|
||
| // let assemblyFormat = [{ | ||
| // $source | ||
| // (custom<DynamicIndexList>($offsets, $const_offsets)^)? | ||
| // (`base_shape` `:` custom<DynamicIndexList>($shape, $const_shape)^ | ||
| // `base_strides` `:` custom<DynamicIndexList>($strides, $const_strides))? | ||
| // attr-dict `:` type($source) `->` qualified(type($TensorDesc)) | ||
| // }]; | ||
|
|
||
| let hasVerifier = 1; | ||
|
|
||
| let hasCustomAssemblyFormat = 1; | ||
|
|
||
| let builders = [ | ||
| OpBuilder<(ins "Type": $tdesc, "TypedValue<MemRefType>": $source)>, | ||
|
|
||
| OpBuilder<(ins "Type": $tdesc, "TypedValue<MemRefType> ": $source, | ||
| "llvm::ArrayRef<OpFoldResult>": $shape, | ||
| "llvm::ArrayRef<OpFoldResult>": $strides)>, | ||
|
|
||
| OpBuilder<(ins "Type": $tdesc, "TypedValue<IntegerType> ": $source, | ||
| "llvm::ArrayRef<OpFoldResult>": $shape, | ||
| "llvm::ArrayRef<OpFoldResult>": $strides)>, | ||
|
|
||
| OpBuilder<(ins "Type": $tdesc, "TypedValue<MemRefType>": $source, | ||
| "llvm::ArrayRef<OpFoldResult>": $offsets)>, | ||
|
|
||
|
|
@@ -163,9 +176,19 @@ def XeGPU_CreateNdDescOp: XeGPU_Op<"create_nd_tdesc", [Pure, ViewLikeOpInterface | |
| } | ||
|
|
||
| ArrayRef<int64_t> getStaticOffsets(){ | ||
| return getConstOffsets(); | ||
| auto attr = getConstOffsetsAttr(); | ||
| if (llvm::isa<IntegerType>(getSourceType()) || attr) | ||
| return attr; | ||
|
|
||
| // The offsets are allowed to be empty. The Traits verification of OffsetSizeAndStrideOpInterface interface assumes offsets being present. So it is set to be MAX to indicate user not passed any value (kDynamic means offsets passed as variable). | ||
| setConstOffsets(llvm::SmallVector<int64_t, 4>(getTensorDescShape().size(), std::numeric_limits<int64_t>::max())); | ||
|
||
| //setConstOffsets(llvm::SmallVector<int64_t, 4>(getTensorDescShape().size(), mlir::ShapedType::kDynamic)); | ||
|
||
|
|
||
| attr = getConstOffsetsAttr(); | ||
| return attr; | ||
| } | ||
|
|
||
|
|
||
| /// wrapper for matching with OffsetSizeAndStrideOpInterface | ||
| /// If source is IntegerType or `const_shape` is filled, | ||
| /// it will return `const_shape`, such that mixes of `shape` | ||
|
|
||
This file contains hidden or 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
This file contains hidden or 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does the Source Type matter here?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For createNdDesc, we have the same check in
getStaticStridesandgetStaticSizes.The op description says the following for the optional
stridesandshape:Both
getStaticStridesandgetStaticSizesrely on a source memref to extract strides and shape, and on a user to provide them alongside ui64.For optional offsets, you propose to use memref shape in one of the comments below, in that case, the check starts to matter. Without a memref, the above quoted op description would also extend to, now optional, offsets.
However, since we still require
shapefor ui64 and we only want to match the rank, we could indirectly depend on the user input viashape:This should work for both memref and ui64 sources
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you try it out on your side? I still run into the same error for ui64 case (if we remove offset[0,0]).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh. Now with some more trying, it works. Thanks!