-
Notifications
You must be signed in to change notification settings - Fork 638
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
[Stream] Implement SpecializeEncodings pass (1/n) #19502
Conversation
compiler/src/iree/compiler/Dialect/Encoding/IR/EncodingAttrs.cpp
Outdated
Show resolved
Hide resolved
using AffinityAnalysisDialectInterface::AffinityAnalysisDialectInterface; | ||
IREE::Stream::LayoutAttrSolverFn | ||
makeLayoutAttrSolver(ModuleOp moduleOp) const { | ||
return [=](IREE::Stream::AffinityAttr aff, Operation *op, |
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.
always use full names for things wherever possible - it's useful hints to readers as to what the type is and how it is treated - shortening to arbitrary forms makes it impossible to find/replace in codebases and more difficult for non-native speakers (here and anywhere else you've shortened things).
return [=](IREE::Stream::AffinityAttr aff, Operation *op, | |
return [=](IREE::Stream::AffinityAttr affinityAttr, Operation *op, |
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.
I see, done.
compiler/src/iree/compiler/Dialect/Stream/IR/StreamInterfaces.h
Outdated
Show resolved
Hide resolved
// TODO(hanchung): We should use the default device in this case. However, | ||
// it is not guaranteed that default device attribute will always be set in | ||
// the IR. (Is the statement correct?) | ||
auto affAttr = affinityOp.getAffinityAttr(); |
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.
as mentioned, affinityAttr
(here and elsewhere)
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.
Done. @benvanik does my comment above make sense? If so, I'm going to remove the (Is the statement correct?)
.
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.
At this point in the IR I don't believe there is a concept of default device, so the whole TODO is not required - in a program with multiple functions there may be many routes to a function through the call graph and we can't locally make decisions about the whole program like that.
compiler/src/iree/compiler/Dialect/Stream/Transforms/SpecializeEncodings.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Dialect/Stream/Transforms/SpecializeEncodings.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Dialect/Stream/Transforms/SpecializeEncodings.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Dialect/Stream/Transforms/SpecializeEncodings.cpp
Show resolved
Hide resolved
compiler/src/iree/compiler/Dialect/Stream/Transforms/SpecializeEncodings.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Dialect/Stream/Transforms/SpecializeEncodings.cpp
Show resolved
Hide resolved
compiler/src/iree/compiler/Dialect/Stream/Transforms/SpecializeEncodings.cpp
Show resolved
Hide resolved
compiler/src/iree/compiler/Dialect/Stream/Transforms/SpecializeEncodings.cpp
Outdated
Show resolved
Hide resolved
@benvanik can you take a second look at this? |
Signed-off-by: hanhanW <[email protected]>
Signed-off-by: hanhanW <[email protected]>
Signed-off-by: hanhanW <[email protected]>
Signed-off-by: hanhanW <[email protected]>
Signed-off-by: hanhanW <[email protected]>
a76fb9a
to
86dd9b0
Compare
Signed-off-by: hanhanW <[email protected]>
There are three major changes in the revision:
AffinityAnalysisDialectInterface
Stream dialect interface. It is used to fetch attributes that are defined by other dialects. In the revision, HAL implements the dialect interface, and it can return whatever attribute attached in HAL::ExecutableTarget attributes. The main idea of the dialect interface is that Stream does not need to depend on HAL to get the layout information.cloneWithLayouts
method to the EncodingAttr. It is used in the encoding specialization pass where it can resolve the layout requirements and add it to thelayouts
field. The other optional parameters are dropped because the layout is already resolved. It can be a new Encoding dialect attribute because it is just describing the layout. The stream tensor ops do not need to know theop_type
,element_types
andoperand_index
parameters. It only needs the layout information, and the attribute should implement the interface method.