Skip to content

Add partially mapped resources tests #476

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.

Sign up for GitHub

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

Open
wants to merge 33 commits into
base: main
Choose a base branch
from
Open

Add partially mapped resources tests #476

Changes from 1 commit
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
f43844d
first attempt
bob80905 Oct 16, 2025
38fde3c
Merge branch 'main' into add_unmapped_resources_tests
bob80905 Oct 16, 2025
d7908c7
Merge branch 'main' into add_unmapped_resources_tests
bob80905 Oct 17, 2025
a3b4ce1
first attempt
bob80905 Oct 17, 2025
2d371ed
self review
bob80905 Oct 17, 2025
284d8fe
add check access fully mapped
bob80905 Oct 17, 2025
9c6664b
make all 3 new functions more consistent between each other
bob80905 Oct 17, 2025
eda2715
rename test file name
bob80905 Oct 17, 2025
b58d924
address variable casing errors
bob80905 Oct 18, 2025
59a0b7c
address Justin and Damyan
bob80905 Oct 21, 2025
d3cbf91
fix build errors
bob80905 Oct 21, 2025
8d9cbbb
attempt to unify SRV
bob80905 Oct 21, 2025
fb4a631
fix texturelayout flag and another flag
bob80905 Oct 22, 2025
1c9752f
add metal XFAIL
bob80905 Oct 22, 2025
3e78f39
improve, but keep UAV the same
bob80905 Oct 24, 2025
fb86727
remove unwanted changes
bob80905 Oct 24, 2025
b9b202b
address confusing comment Justin pointed out
bob80905 Oct 24, 2025
361225b
clang-format
bob80905 Oct 24, 2025
eb89f86
address damyan, xfail intel
bob80905 Oct 24, 2025
c59e9e2
remove reserved CBV function, and capitalize var names
bob80905 Oct 24, 2025
8579b37
Merge branch 'main' into add_unmapped_resources_tests
bob80905 Oct 24, 2025
6a7878d
add some missing constts
bob80905 Oct 24, 2025
26f1045
Merge branch 'add_unmapped_resources_tests' of https://github.com/bob…
bob80905 Oct 24, 2025
06bf89c
self review
bob80905 Oct 25, 2025
e7f9a91
address Sarah
bob80905 Nov 6, 2025
2b2b9aa
add overflow assert
bob80905 Nov 7, 2025
7b8fa4c
fix assert condition, and update resource elty
bob80905 Nov 10, 2025
b21a1c6
add back accidentally removed XFAIL
bob80905 Nov 11, 2025
b6effce
update test
bob80905 Nov 11, 2025
ffff4eb
validate 0's return behavior
bob80905 Nov 11, 2025
0dd409e
use bool vals instead of ints
bob80905 Nov 11, 2025
84e9d82
remove 0's check, just assign if cafm returns false
bob80905 Nov 11, 2025
a861854
use 1 and 0 rather than true and false to resolve error
bob80905 Nov 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion lib/API/DX/Device.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,11 +532,14 @@ class DXDevice : public offloadtest::Device {
UINT Ret;
if (NumTiles.has_value())
Ret = static_cast<UINT>(*NumTiles);
else
else {
// Map the entire buffer by computing how many 64KB tiles cover it
Ret = static_cast<UINT>(
(Width + D3D12_TILED_RESOURCE_TILE_SIZE_IN_BYTES - 1) /
Copy link
Collaborator

@spall spall Oct 29, 2025

Choose a reason for hiding this comment

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

could this operation overflow? Why is width a uint64?

Copy link
Contributor Author

@bob80905 bob80905 Nov 6, 2025

Choose a reason for hiding this comment

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

Currently the arg that feeds into the width parameter is an int datatype. So it doesn't need to be a uint64, at least not yet. I'll change it.

Copy link
Collaborator

@spall spall Nov 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

could width or d3d12_tiled_resource_tile_size_in_bytes be uint32 max or half that amount; could this operation overflow after you change width to be a uint32?

Copy link
Contributor Author

@bob80905 bob80905 Nov 7, 2025

Choose a reason for hiding this comment

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

I don't believe D3D12_TILED_RESOURCE_TILE_SIZE_IN_BYTES can be any larger than 65536. However, width could be any arbitrary size, depending on how the developer specified it. So yes, I think this operation could overflow, though in exceedingly specific and pathological circumstances.

Copy link
Collaborator

@damyanp damyanp Nov 7, 2025

Choose a reason for hiding this comment

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

FWIW: in D3D12, Width is a uint64 because it needs to be able to support describing buffers that are larger than 4gb.

Copy link
Contributor Author

@bob80905 bob80905 Nov 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, though for this API:
UpdateTileMappings
The pRangeTileCounts param is a UINT. Its value in my code comes directly from counting the number of tiles. So I don't think this should ever be > uint32_max.

D3D12_TILED_RESOURCE_TILE_SIZE_IN_BYTES);
// check for overflow
assert(Ret < Width || Ret < D3D12_TILED_RESOURCE_TILE_SIZE_IN_BYTES);
}
return Ret;
}

Expand Down
Loading