This is an archive of the discontinued LLVM Phabricator instance.

[NVPTX] Use by default 'sm_60' architecture when expanding %ptxas-verify macro.
ClosedPublic

Authored by pavelkopyl on Jan 13 2023, 3:52 PM.

Details

Summary

Also get rid of explicitly specified '-march' values for old architectures.
This simplifies %ptxas-verify statements.
After the change, we can potentially miss cases where a new functionality
is added to the architecture without appropriate checks in the
backend. On the other hand, this is mostly true for old architectures
that have been thoroughly tested.

Diff Detail

Event Timeline

pavelkopyl created this revision.Jan 13 2023, 3:52 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 13 2023, 3:52 PM
pavelkopyl requested review of this revision.Jan 13 2023, 3:52 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 13 2023, 3:52 PM
tra added inline comments.Jan 13 2023, 5:10 PM
llvm/test/lit.cfg.py
239

This will be a problem for cases where we generate PTX for a newer GPU variants.
At the very least users need to be able to override -arch=... passed to ptxas-verify.

pavelkopyl added inline comments.Feb 14 2023, 9:17 AM
llvm/test/lit.cfg.py
239

I agree, but I guess overriding shouldn't be a problem. For example,

%{ llc < %s -march=nvptx63 -mcpu=sm_80 | %ptxas-verify -arch=sm_80 %}

gets expanded into:

... | /usr/local/cuda-12/bin/ptxas -arch=sm_60 -c - -arch=sm_80

Ptxas warns on that:

"ptxas warning : incompatible redefinition for option 'gpu-name', the last value of this option was used".

So, the last

-arch=...

option has a precedence over the previous ones. This is what we need. The warning message is dropped by /dev/null.

tra accepted this revision.Feb 14 2023, 10:42 AM

LGTM.

llvm/test/lit.cfg.py
239

It may be a good idea to update lit docs and document that the default for #ptxas/%ptxas-verify is sm_60 now.

This revision is now accepted and ready to land.Feb 14 2023, 10:42 AM
pavelkopyl added inline comments.Feb 15 2023, 3:35 PM
llvm/test/lit.cfg.py
239

I'd like to document this, but it's not clear where to put comments exactly. I added them to llvm/test/lit.cfg.py. Is that OK?

  • Add a comment that %ptxas-verify defaults to sm_60 arch.
pavelkopyl retitled this revision from [NVPTX] Use 'sm_60' architecture when expanding %ptxas-verify macro. to [NVPTX] Use by default 'sm_60' architecture when expanding %ptxas-verify macro..Feb 15 2023, 3:37 PM
tra accepted this revision.Feb 15 2023, 4:01 PM

it's not clear where to put comments exactly. I added them to llvm/test/lit.cfg.py. Is that OK?

I don't have any better ideas. lit.cfg.py seems to be a reasonable place for it.