Skip to content

Warning if cbar synchronizes across workgroups#42

Open
natgavrilenko wants to merge 1 commit intoKhronosGroup:mainfrom
natgavrilenko:cbar-tests
Open

Warning if cbar synchronizes across workgroups#42
natgavrilenko wants to merge 1 commit intoKhronosGroup:mainfrom
natgavrilenko:cbar-tests

Conversation

@natgavrilenko
Copy link
Contributor

Print a warning if control barriers with the same instance id are used across different workgroups. Update existing tests with such barriers to use a smaller scope.

o << " no ("; sequenceInSuffix(o, 0, instnum+1, true, ") & X.swg");
firstInstInWG = instnum+1;
}
workgroupnum++;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should also increment this for "NEWQF"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I can tell from the existing tests, NEWQF is always followed by NEWWG, so incrementing the counter for both would cause it to increment twice. Also, the threadnum counter is only incremented for NEWTHREAD.

But both versions should work, since the exact workgroup ID doesn’t really matter, as long as all threads in the same workgroup get the same workgroup ID.

NEWWG
NEWSG
NEWTHREAD
st.av.scopedev.sc0 x = 1
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO we should try to keep the original scopes, but replace cbar with atomics to synchronize across workgroups.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Print a warning if control barriers with the same instance id are
used across different workgroups. Update existing tests with such
barriers to use acquire-release memory accesses instead.

Signed-off-by: Natalia Gavrilenko <natalia.gavrilenko@huawei.com>
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