[PATCH] D60620: [HIP] Support target id by --offload-arch

This revision was automatically updated to reflect the committed changes. Closed by commit rG7546b29e7616: [HIP] Support target id by --offload-arch (authored by yaxunl). Herald added a project: clang. Changed prior to commit: https://reviews.llvm.org/D60620?vs=285482&id=286469#toc Repository:

[PATCH] D60620: [HIP] Support target id by --offload-arch

yaxunl marked 3 inline comments as done. yaxunl added a comment. In D60620#2216796 , @tra wrote: > Couple of minor nits. LGTM otherwise. Will revise as suggested when committing. Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60620/new/

[PATCH] D60620: [HIP] Support target id by --offload-arch

tra accepted this revision. tra added a comment. This revision is now accepted and ready to land. Couple of minor nits. LGTM otherwise. Comment at: clang/include/clang/Basic/TargetID.h:42 +/// is not a null pointer. +/// If \p CanonicalizeProc is true, canonicalize returned pro

[PATCH] D60620: [HIP] Support target id by --offload-arch

yaxunl added inline comments. Comment at: clang/include/clang/Basic/TargetID.h:42 +/// is not a null pointer. +/// If \p CanonicalizeProc is true, canonicalize returned processor name. +llvm::Optional tra wrote: > Comment needs updating as parameters and return v

[PATCH] D60620: [HIP] Support target id by --offload-arch

yaxunl updated this revision to Diff 285482. yaxunl marked 13 inline comments as done. yaxunl added a comment. revised by Artem's comments, with minor bug fixes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60620/new/ https://reviews.llvm.org/D60620 Files: clang/include/clang/Basic/

[PATCH] D60620: [HIP] Support target id by --offload-arch

tra added a comment. Looks good in general. Mostly C++ style comments below. Comment at: clang/include/clang/Basic/TargetID.h:30 +/// Returns canonical processor name or empty if the processor name is invalid. +llvm::StringRef getProcessorFromTargetID(const llvm::Triple &T, +

[PATCH] D60620: [HIP] Support target id by --offload-arch

yaxunl updated this revision to Diff 284882. yaxunl added a comment. rebase to ToT and minor bug fixes CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60620/new/ https://reviews.llvm.org/D60620 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/include/clang/Basic/Target

[PATCH] D60620: [HIP] Support target id by --offload-arch

yaxunl updated this revision to Diff 281465. yaxunl added a comment. separate emitting target-id module flag to a different patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60620/new/ https://reviews.llvm.org/D60620 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clan

[PATCH] D60620: [HIP] Support target id by --offload-arch

yaxunl added a comment. @tra target ID documentation is added by https://reviews.llvm.org/D84822 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60620/new/ https://reviews.llvm.org/D60620 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D60620: [HIP] Support target id by --offload-arch

yaxunl updated this revision to Diff 280567. yaxunl marked 9 inline comments as done. yaxunl added a comment. revised by Artem's comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60620/new/ https://reviews.llvm.org/D60620 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td

[PATCH] D60620: [HIP] Support target id by --offload-arch

yaxunl marked 20 inline comments as done. yaxunl added inline comments. Comment at: clang/include/clang/Driver/Driver.h:332 + llvm::Triple getHIPOffloadTargetTriple() const; + tra wrote: > This is used exclusively by the Driver.cpp and does not have to be a pu

[PATCH] D60620: [HIP] Support target id by --offload-arch

tra added inline comments. Comment at: clang/include/clang/Driver/Driver.h:332 + llvm::Triple getHIPOffloadTargetTriple() const; + This is used exclusively by the Driver.cpp and does not have to be a public API call. Comment at: clang/lib/B

[PATCH] D60620: [HIP] Support target id by --offload-arch

yaxunl updated this revision to Diff 279000. yaxunl added a comment. Herald added subscribers: llvm-commits, dang, hiraditya. Herald added a project: LLVM. rebase and added more checks. The documentation work is still under development. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D6062

[PATCH] D60620: [HIP] Support target id by --offload-arch

tra added a comment. In D60620#2089722 , @yaxunl wrote: > In D60620#2067134 , @tra wrote: > > > Do you expect users to specify these IDs? How do you see it being used in > > practice? I think you do need to impleme

[PATCH] D60620: [HIP] Support target id by --offload-arch

yaxunl added a comment. In D60620#2067134 , @tra wrote: > Do you expect users to specify these IDs? How do you see it being used in > practice? I think you do need to implement a user-friendly shortcut and > expand it to the detailed offload-id internall

[PATCH] D60620: [HIP] Support target id by --offload-arch

tra added a comment. In D60620#2064839 , @yaxunl wrote: > It means HIP will create two compilation passes: one for gfx908 and one for > gfx908:xnack+:sramecc+. OK, so empty feature list may also be valid. > > >> One thing you'll need is a way to norm

[PATCH] D60620: [HIP] Support target id by --offload-arch

yaxunl updated this revision to Diff 267693. yaxunl added a comment. emit empty target id module flag if no -target-cpu is set CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60620/new/ https://reviews.llvm.org/D60620 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/in

[PATCH] D60620: [HIP] Support target id by --offload-arch

yaxunl added a comment. > // ... > // RUN: -x hip --offload-arch=gfx908 \ > // RUN: --offload-arch=gfx908:sramecc+:xnack+ > Does this mean that HIP will create two compilation passes -- one for gfx908 > and one for gfx908:sramecc+:xnack+ ? > Or does it mean that the first line is ignored if y

[PATCH] D60620: [HIP] Support target id by --offload-arch

yaxunl updated this revision to Diff 266976. yaxunl added a comment. Emit target id module flag metadata. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60620/new/ https://reviews.llvm.org/D60620 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/include/clang/Basic/Off

[PATCH] D60620: [HIP] Support target id by --offload-arch

tra added inline comments. Comment at: clang/lib/Driver/ToolChains/HIP.cpp:121-123 + auto Pos = SubArchName.find_first_of("+-"); + if (Pos != SubArchName.npos) +SubArchName = SubArchName.substr(0, Pos); yaxunl wrote: > tra wrote: > > yaxunl wrote: > > > tra

[PATCH] D60620: [HIP] Support target id by --offload-arch

yaxunl updated this revision to Diff 266410. yaxunl added a comment. Changed target id format to be like `gfx908:xnack+:sramecc-`. I tried to introduce --offload-target-id but found that is not good because: 1. it will cause redundant code since I have to handle these options separately in CUDA

[PATCH] D60620: [HIP] Support target id by --offload-arch

yaxunl marked an inline comment as done. yaxunl added inline comments. Comment at: clang/lib/Driver/ToolChains/HIP.cpp:121-123 + auto Pos = SubArchName.find_first_of("+-"); + if (Pos != SubArchName.npos) +SubArchName = SubArchName.substr(0, Pos); tra wrote:

[PATCH] D60620: [HIP] Support target id by --offload-arch

tra added inline comments. Comment at: clang/lib/Driver/ToolChains/HIP.cpp:121-123 + auto Pos = SubArchName.find_first_of("+-"); + if (Pos != SubArchName.npos) +SubArchName = SubArchName.substr(0, Pos); yaxunl wrote: > tra wrote: > > Parsing should probably

[PATCH] D60620: [HIP] Support target id by --offload-arch

yaxunl updated this revision to Diff 266315. yaxunl marked an inline comment as done. yaxunl added a comment. Herald added subscribers: kerbowa, nhaehnle, jvesely. Fixed passing target id to clang -cc1. Added predefined macros for target id. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D60620: [HIP] Support target id by --offload-arch

yaxunl marked 3 inline comments as done. yaxunl added inline comments. Comment at: clang/lib/Basic/HIP.cpp:16 +const llvm::SmallVector +getAllPossibleTargetIdFeatures(llvm::StringRef Device) { + llvm::SmallVector Ret; tra wrote: > Nit: there's an unfortunate cla

[PATCH] D60620: [HIP] Support target id by --offload-arch

yaxunl marked 2 inline comments as done. yaxunl added inline comments. Comment at: clang/lib/Driver/ToolChains/HIP.cpp:121-123 + auto Pos = SubArchName.find_first_of("+-"); + if (Pos != SubArchName.npos) +SubArchName = SubArchName.substr(0, Pos); tra wrote:

[PATCH] D60620: [HIP] Support target id by --offload-arch

tra added inline comments. Comment at: clang/lib/Basic/HIP.cpp:16 +const llvm::SmallVector +getAllPossibleTargetIdFeatures(llvm::StringRef Device) { + llvm::SmallVector Ret; Nit: there's an unfortunate clash with already [[ https://github.com/llvm/llvm-project/

[PATCH] D60620: [HIP] Support target id by --offload-arch

yaxunl added a comment. In D60620#1464633 , @tra wrote: > It looks like you are solving two problems here. > a) you want to create multiple device passes for the same GPU, but with > different options. > b) you may want to pass different compiler option

[PATCH] D60620: [HIP] Support target id by --offload-arch

yaxunl updated this revision to Diff 265025. yaxunl retitled this revision from "[HIP] Support -offloading-target-id" to "[HIP] Support target id by --offload-arch". yaxunl edited the summary of this revision. yaxunl added a comment. rebased the patch and revised by passing target id by `--offloa