-
Notifications
You must be signed in to change notification settings - Fork 14.6k
[mlir][spirv] Add 8-bit float type emulation #148811
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
Changes from all commits
4b6d70d
4bdd204
6a3d341
a656137
730bace
4167a4a
357c290
9ce85ef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,6 +39,10 @@ struct SPIRVConversionOptions { | |
/// The number of bits to store a boolean value. | ||
unsigned boolNumBits{8}; | ||
|
||
/// Whether to emulate unsupported floats with integer types of same bit | ||
/// width. | ||
bool emulateUnsupportedFloatTypes{true}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you sure we want this on by default? I think this can be a footgun when users inadvertently use unsupported fp types and get a dialect conversion error over integer types down the line... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd rather this was opt-in unless we have good justification for keeping this opt-out There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the theory is that you'll generally want to do software emulation of the small floats earlier? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm concerned about cases when you ended up with unsupported types by accident -- this often comes up with all the variants of fp8 and smaller fp types that are present in the input MLIR at the level of linalg/arith. This is much easier to diagnose when you can see the original type. IMO, dialect conversion should error out by default, unless someone opts into these types being handled in some other way. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LLVM precedent is to just do the f8* => i8, though? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. llvm ir or convert-to-llvm? llvm has the same issue as spirv here that its type system has fewer primitive types than mlir, so it's on mlir to figure out how to handle unsupported types There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, so we can leave this on by default to match llvm conversion if this is the case There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, @krzysz00 . Yes, I was following the llvm precedence. |
||
|
||
/// How sub-byte values are storaged in memory. | ||
SPIRVSubByteTypeStorage subByteTypeStorage{SPIRVSubByteTypeStorage::Packed}; | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.