This is an archive of the discontinued LLVM Phabricator instance.

Corrected fragment size for tf32 LD B matrix.
ClosedPublic

Authored by JackAKirk on Jan 24 2022, 2:19 AM.

Details

Summary

Signed-off-by: JackAKirk <jack.kirk@codeplay.com>

Diff Detail

Event Timeline

JackAKirk requested review of this revision.Jan 24 2022, 2:19 AM
JackAKirk created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJan 24 2022, 2:19 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Note that the test, llvm/test/CodeGen/NVPTX/wmma.py line 210, had the correct value already but didn't seem to cover the mistake.

tra accepted this revision.Jan 24 2022, 10:47 AM

LGTM. Should I commit the patch on your behalf?

Note that the test, llvm/test/CodeGen/NVPTX/wmma.py line 210, had the correct value already but didn't seem to cover the mistake.

It appears that we do not actually test for the correct number of registers used in an instruction argument, only for the correct instruction variant itself.
It should've been caught if we were to attempt assembling generated PTX with ptxas.

This revision is now accepted and ready to land.Jan 24 2022, 10:47 AM

LGTM. Should I commit the patch on your behalf?

Note that the test, llvm/test/CodeGen/NVPTX/wmma.py line 210, had the correct value already but didn't seem to cover the mistake.

It appears that we do not actually test for the correct number of registers used in an instruction argument, only for the correct instruction variant itself.
It should've been caught if we were to attempt assembling generated PTX with ptxas.

Thanks I can try to land it myself. I executed arc land and it is asking me to "Land these changes". I am not sure that I have the permissions for this to succeed, but I can try. I have not done this before: is it safe to enter simply "y" with the existing commit message (name/email in the commit are correct)?

tra added a comment.Jan 24 2022, 11:18 AM

If you are not sure if you have commit access, you probably do not. It must be explicitly granted:
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access

As for landing the patch, I do it manually with git. Never tried it with arc, so I can't say much, except that it will not work until you have commit access.

If you are not sure if you have commit access, you probably do not. It must be explicitly granted:
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access

As for landing the patch, I do it manually with git. Never tried it with arc, so I can't say much, except that it will not work until you have commit access.

Ah OK. In that case then yes please can you land it for me?

tra added a comment.Jan 24 2022, 12:35 PM

The patch uses a @gmail.com email. Should I change it to JackAKirk <jack.kirk@codeplay.com> to match the sign-off email?

The patch uses a @gmail.com email. Should I change it to JackAKirk <jack.kirk@codeplay.com> to match the sign-off email?

Ah yes thanks for catching that. Please use JackAKirk <jack.kirk@codeplay.com>.

This revision was landed with ongoing or failed builds.Jan 25 2022, 11:31 AM
This revision was automatically updated to reflect the committed changes.