[PATCH] D135614: [OpenMP][CUDA][AMDGPU] Accept case insensitive subarchitecture names
Johannes Doerfert via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 13 16:25:08 PDT 2022
jdoerfert added a comment.
First, as noted, I asked @jhuber6 to update this.
In D135614#3847775 <https://reviews.llvm.org/D135614#3847775>, @tra wrote:
> Is that really something we need/want to do? I've never seen anyone complaining about this particular issue.
I have, and not only once. Hence the request for a change.
> clang/gcc are case-sensitive for similar options, like `-march`: https://godbolt.org/z/3jKzTda7h
> I think offloading options should behave consistently in that regard. If we do want to make arch/CPU names case-agnostic, it should be done for all such options.
I don't disagree. Major difference is though that march returns a nice list of options in your example.
For offload-arch the situations is less user friendly: https://godbolt.org/z/oWYvvjTTq
In D135614#3849656 <https://reviews.llvm.org/D135614#3849656>, @b-sumner wrote:
> Also, we may want to use uppercase for other purposes in the future.
gfx90a != gfx90A but both valid? Please, no.
In D135614#3849635 <https://reviews.llvm.org/D135614#3849635>, @yaxunl wrote:
> I am not sure whether it is a good idea to allow gfx90A in `--offload-arch`, since it is not documented in LLVM AMDGPU usage. @b-sumner @arsenm
I'm not sure which documentation you mean but if it's https://llvm.org/docs/AMDGPUUsage.html, then I doubt any "end user" will have read it.
Arguably it's for compiler writers and not even in the frontend documentation. Maybe there is something else I don't know about?
In D135614#3849639 <https://reviews.llvm.org/D135614#3849639>, @arsenm wrote:
> I don't really have an opinion here. I'd probably lean towards a "did you mean" kind of warning
Since listing all options might not be great, I agree that we could emit a warning for upper/lower case situations instead. (FWIW, not only for amdgpu btw.)
@tra, would that be OK for you?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135614/new/
https://reviews.llvm.org/D135614
More information about the llvm-commits
mailing list