Skip to content

fix incorrect tyep casting for workgroup_max_dim and grid_max_dim - #6

Merged
cfreehill merged 2 commits into
ROCm:masterfrom
scchan:fix_group_max_dim
May 9, 2018
Merged

cfreehill merged 2 commits into
ROCm:masterfrom
scchan:fix_group_max_dim

Conversation

@scchan

@scchan scchan commented May 8, 2018

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread rocminfo.cc Outdated
}
printLabelInt("Dim[0]", agent_i->grid_max_dim.x, 2);
printLabelInt("Dim[1]", agent_i->grid_max_dim.y, 2);
printLabelInt("Dim[2]", agent_i->grid_max_dim.z, 2);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If you are going to unroll the loop, you may as well use the HSA naming ("x, y z") instead of "Dim[0], Dim[1], Dim[2]"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

For the workgroup max sizes for both the agent the isa info, it's using the "Dim[n]" naming. I don't have any strong preference between "x,y,z" and "Dim[n]" but I think they should be consistent.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I agree, consistency is most important. Since the documentation refers to (x, y, z), then I would go with that (which means not using a loop in the workgroup cases too)...assuming this doesn't cause any complications. If the "Dim[]" is somehow easier than "x, y, z" on tools that parse this output, then leaving it as-is is fine.

Comment thread rocminfo.cc Outdated
printLabelInt("z", isa_i->grid_max_dim.z, indent+1);
printLabel("Grid Max Size per Dimension:", true, indent);
printLabelInt("Dim[0]", isa_i->grid_max_dim.x, indent+1);
printLabelInt("Dim[1]", isa_i->grid_max_dim.y, indent+1);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is there a reason to switch to the Dim[] naming vs. "x, y, z"? The latter was based on HSA docs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

see comment for the agent max grid size

print out the max grid/group size before the "per Dimension" info
@cfreehill
cfreehill merged commit 8d579bf into ROCm:master May 9, 2018
rahulc-gh pushed a commit that referenced this pull request Mar 26, 2025
Change-Id: Id0720988fb01079a3ca6f3287dc743d93db4fd0f
rahulc-gh pushed a commit that referenced this pull request Apr 9, 2025
* Obey CMAKE_BUILD_TYPE

Obey CMAKE_BUILD_TYPE when it is passed on the command line. Previous
behavior remains:

Debug by default
ROCRTST_BLD_TYPE can set build type

* External CI: change trigger from amd-master to amd-mainline

* Added gfx94x and gfx11xx  IDs to enumerator (#5)

* Use idomatic approach to extending CMAKE_MODULE_PATH. (#7)

There are many reasons why there may already be a CMAKE_MODULE_PATH defined.
The idiomatic way to extend it in a project is via list(APPEND).

#97 submitted by @stellaraccident

Co-authored-by: Stella Laurenzo <Stella.Laurenzo@amd.com>

* Added release trigger for further releases

* Fix for typo in ISA_TO_ID table (#6)

Change-Id: Id0720988fb01079a3ca6f3287dc743d93db4fd0f

* Remove WSL checks for displaying UUID and BDFID

Signed-off-by: lyndonli <Lyndon.Li@amd.com>

* Added KWS check for amd-mainline (#9)

---------

Signed-off-by: lyndonli <Lyndon.Li@amd.com>
Co-authored-by: benrichard-amd <ben.richard@amd.com>
Co-authored-by: Su, Daniel <Daniel.Su@amd.com>
Co-authored-by: Stella Laurenzo <Stella.Laurenzo@amd.com>
Co-authored-by: Mallya, Ameya Keshava <AmeyaKeshava.Mallya@amd.com>
Co-authored-by: Yat Sin, David <David.YatSin@amd.com>
Co-authored-by: lyndonli <Lyndon.Li@amd.com>
ammallya pushed a commit that referenced this pull request Oct 28, 2025
Change-Id: Id0720988fb01079a3ca6f3287dc743d93db4fd0f


[ROCm/rocminfo commit: ac6000d]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants