-
Notifications
You must be signed in to change notification settings - Fork 34
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
[Lang] Revisit memory model #321
Open
llvm-beanz
wants to merge
9
commits into
microsoft:main
Choose a base branch
from
llvm-beanz:cbieneman/memory-terms
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
a483c45
[Lang] Revisit memory model
llvm-beanz 0040940
Update specs/language/introduction.tex
llvm-beanz 1ce6657
Update specs/language/introduction.tex
llvm-beanz 809bbc5
Update specs/language/introduction.tex
llvm-beanz e66e423
bytes->bits
llvm-beanz 2398305
Remove object terms, those should go in a separate section
llvm-beanz d89823f
Fix up the footnote based on feedback from @damyanp
llvm-beanz 5f062b8
Working on clarifying the spec language based on feedback.
llvm-beanz 0516676
Adding missing placeholder
llvm-beanz 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 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 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.
There are a few constraints around memory accesses in HLSL and DXIL that you're trying to abstract over here, but I'm not sure the "line width" idea captures them effectively. In some sense it might seem nice to boil down some similar rules into a simple concept, but it's worth noting why the rules are what they are and how they might change.
Data access via TypedBuffer. This is presumably where the "line width" idea comes from, but a lot of its complexity is unnecessary if we disallow "types that happen to fit" as type arguments to
Buffer<>
. Here, we have accesses to typed buffers and textures, and the operation that accesses them operates on a 4-element contained type. ABuffer<float>
is really aBuffer<float4>
that we only use one element from.This gets a bit confusing for 64 bit types. Notably,
Buffer<double4>
is not valid HLSL. However, this is really an implementation detail leaking through since the storage actually splits doubles up into int32 parts. So it's probably better to just think ofBuffer<double2>
as syntactic sugar for the casts and just call this kind of memory access what it is - access into a container of 4 at-most 32-bit values.Vectors of more than 4 elements don't exist in HLSL. This is simply due to the fact that there's a fixed set of vector types and no way for a user to create their own. It isn't a meaningful rule, and in spaces like local device memory we really don't need any constraints on the language here. If it's possible to write a
double8
somehow in the future in the language and that isn't used in constant or typed buffers specifically, it's straightforward for implementations to do whatever they need to do to lower it. I don't think we want to define an artificial limit there.So I guess TLDR I think we should simply say two separate things rather than trying to define "line width":
Also note that I use "constant buffer" memory in my wording above, rather than "constant memory". We may want to keep that terminology available for if we ever do something in that space that doesn't carry the constant buffer legacy.
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 agree.
I think part of the problem is trying to view all resource accesses as if they are like native memory accesses from the shader, with only the address space placing constraints on alignment and such. I think we can evolve constant memory and raw/structured buffer memory in this direction, but not typed/texture accesses. For memory that goes in this direction, I don't think "line width" would be a concept we want to use/keep, and "minimum alignment" will be defined in other ways, rather than by some fixed value applied to a memory type.
Some notes:
I don't think I would agree with that. First, it's a confusing use of the term "element" here. Perhaps you had a different definition of "element" in mind than what I am interpreting here, but I struggle to think of a single definition that fits into this statement. Plenty of elements of structures and arrays in HLSL that exceed 128-bits in size can be placed into the constant buffer. You can declare a double4 (or array of such) in a constant buffer, which will use two rows for the vector. It's just that structures, array elements, and any type that cannot fit within the remainder of a row will be started at the beginning of the next available row. For some of these, that's part of the high-level packing rules, not necessarily something intrinsic to the DXIL interface. For array elements, they must be 128-bit aligned to ensure that array indexing maps to an index in the DXIL legacy constant buffer load op without impacting the index of the component read from the result.
For legacy constant buffer load in DXIL, it's important to note that this load op doesn't mean all of the components are loaded - only the components that are extracted from the result structure need to be loaded. It's a subtle difference, but important in certain circumstances, and mismatches the concept of "line width" as applied to constant buffers. Think of the DXIL op as a compromise as there wasn't an easy way to express the thing that's expressed easily in DXBC asm like so:
CB0[0][0].yyyz
(only loadsy
andz
components).