[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

jhuber6 added a comment. In D153725#4485039 , @arsenm wrote: > And the libomptarget build is in fact doing that, but it shouldn't have to. > What it's doing actually seems really unreasonable. It's only building the > locally found targets when it shoul

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

JonChesterfield added a comment. The test detection is an awkward compromise between people who want to run the GPU tests and people who don't, and reflects diverse hardware in use and variation on whether cuda / hsa are installed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

JonChesterfield added a comment. In D153725#4484966 , @arsenm wrote: > In D153725#4484754 , > @JonChesterfield wrote: > >> - if you open the driver too many times at once it fails to open, so running >> a parall

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

arsenm added a comment. In D153725#4484973 , @arsenm wrote: > In D153725#4484754 , > @JonChesterfield wrote: > >> The problem with using the proper API via HSA or similar is twofold: >> >> - we use this tool to e

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

jhuber6 added a comment. In D153725#4484966 , @arsenm wrote: > In D153725#4484754 , > @JonChesterfield wrote: > >> - if you open the driver too many times at once it fails to open, so running >> a parallel build

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

arsenm added a comment. In D153725#4484754 , @JonChesterfield wrote: > The problem with using the proper API via HSA or similar is twofold: > > - we use this tool to enable tests, which means HSA has to exist before > building clang or the tests don't r

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

arsenm added a comment. In D153725#4484754 , @JonChesterfield wrote: > - if you open the driver too many times at once it fails to open, so running > a parallel build that uses this tool doesn't work on fast machines Why would this happen? Seems like a

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

JonChesterfield added a comment. In D153725#4484747 , @arsenm wrote: > In D153725#4484711 , > @JonChesterfield wrote: > >> The right thing to do on Linux for this is to query the driver directly. >> That is, the

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

arsenm added inline comments. Comment at: clang/tools/amdgpu-arch/AMDGPUArchByHIP.cpp:80 +if (err != hipSuccess) { + llvm::errs() << "Failed to get device id for ordinal " << i << "\n"; + return 1; yaxunl wrote: > arsenm wrote: > > single quotes aro

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

arsenm added a comment. In D153725#4484711 , @JonChesterfield wrote: > The right thing to do on Linux for this is to query the driver directly. That > is, the kernel should populate some string under /sys that we read. That > isn't yet implemented. It

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

JonChesterfield added inline comments. Comment at: clang/tools/amdgpu-arch/AMDGPUArch.cpp:50 +#else + return printGPUsByHSA(); +#endif yaxunl wrote: > jhuber6 wrote: > > arsenm wrote: > > > The HIP path should work on linux too. I generally think we should build

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

JonChesterfield added a comment. The right thing to do on Linux for this is to query the driver directly. That is, the kernel should populate some string under /sys that we read. That isn't yet implemented. Does windows happen to have that functionality available? (landed here while trying to w

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. yaxunl marked an inline comment as done. Closed by commit rG661d91a0fd4a: [clang] Make amdgpu-arch tool work on Windows (authored by yaxunl). Herald added a project: cl

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

jhuber6 accepted this revision. jhuber6 added a comment. This revision is now accepted and ready to land. LG CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153725/new/ https://reviews.llvm.org/D153725 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

yaxunl marked an inline comment as done. yaxunl added inline comments. Comment at: clang/tools/amdgpu-arch/AMDGPUArchByHIP.cpp:96 + return 0; +} jdoerfert wrote: > Where is the call to this? forgot to call this in main. fixed. CHANGES SINCE LAST ACTION https

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

yaxunl updated this revision to Diff 538207. yaxunl marked an inline comment as done. yaxunl added a comment. revised by comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153725/new/ https://reviews.llvm.org/D153725 Files: clang/tools/amdgpu-arch/AMDGPUArch.cpp clang/tools/amd

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

jdoerfert added inline comments. Comment at: clang/tools/amdgpu-arch/AMDGPUArchByHIP.cpp:96 + return 0; +} Where is the call to this? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153725/new/ https://reviews.llvm.org/D153725 _

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

yaxunl marked an inline comment as done. yaxunl added inline comments. Comment at: clang/tools/amdgpu-arch/AMDGPUArch.cpp:48-52 + if (!printGPUsByHSA()) +return 0; +#endif + return printGPUsByHSA(); jhuber6 wrote: > Are we missing something here ? They lo

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

jhuber6 added inline comments. Comment at: clang/tools/amdgpu-arch/AMDGPUArch.cpp:48-52 + if (!printGPUsByHSA()) +return 0; +#endif + return printGPUsByHSA(); Are we missing something here ? They look the same. CHANGES SINCE LAST ACTION https://review

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

yaxunl marked 5 inline comments as done. yaxunl added inline comments. Comment at: clang/tools/amdgpu-arch/AMDGPUArch.cpp:50 +#else + return printGPUsByHSA(); +#endif jhuber6 wrote: > arsenm wrote: > > The HIP path should work on linux too. I generally think we

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

yaxunl updated this revision to Diff 538170. yaxunl marked an inline comment as done. yaxunl added a comment. revised by comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153725/new/ https://reviews.llvm.org/D153725 Files: clang/tools/amdgpu-arch/AMDGPUArch.cpp clang/tools/amd

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

jhuber6 added inline comments. Comment at: clang/tools/amdgpu-arch/AMDGPUArch.cpp:50 +#else + return printGPUsByHSA(); +#endif arsenm wrote: > The HIP path should work on linux too. I generally think we should build as > much code as possible on all hosts, so h

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

arsenm added inline comments. Comment at: clang/tools/amdgpu-arch/AMDGPUArch.cpp:50 +#else + return printGPUsByHSA(); +#endif The HIP path should work on linux too. I generally think we should build as much code as possible on all hosts, so how about ``` #ifnde

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

jhuber6 added inline comments. Comment at: clang/tools/amdgpu-arch/AMDGPUArch.cpp:47 - // Attempt to load the HSA runtime. - if (llvm::Error Err = loadHSA()) { -logAllUnhandledErrors(std::move(Err), llvm::errs()); -return 1; - } - - hsa_status_t Status = hsa_init();

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

jhuber6 added inline comments. Comment at: clang/tools/amdgpu-arch/AMDGPUArch.cpp:47 - // Attempt to load the HSA runtime. - if (llvm::Error Err = loadHSA()) { -logAllUnhandledErrors(std::move(Err), llvm::errs()); -return 1; - } - - hsa_status_t Status = hsa_init();

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

arsenm added inline comments. Comment at: clang/tools/amdgpu-arch/AMDGPUArch.cpp:47 - // Attempt to load the HSA runtime. - if (llvm::Error Err = loadHSA()) { -logAllUnhandledErrors(std::move(Err), llvm::errs()); -return 1; - } - - hsa_status_t Status = hsa_init();

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

jhuber6 added a comment. Also w.r.t. target-id, I'm wondering what a good solution would be. Right now the main usage of `amdgpu-arch` is both to detect the `-mcpu / -march` in CMake and to fill in the architecture via `--offload-arch=native` or `-fopenmp-target=amdgcn-amd-amdhsa`. We may want

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

yaxunl marked 2 inline comments as done. yaxunl added inline comments. Comment at: clang/tools/amdgpu-arch/AMDGPUArch.cpp:47 - // Attempt to load the HSA runtime. - if (llvm::Error Err = loadHSA()) { -logAllUnhandledErrors(std::move(Err), llvm::errs()); -return 1; -

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

jhuber6 added inline comments. Comment at: clang/tools/amdgpu-arch/AMDGPUArch.cpp:47 - // Attempt to load the HSA runtime. - if (llvm::Error Err = loadHSA()) { -logAllUnhandledErrors(std::move(Err), llvm::errs()); -return 1; - } - - hsa_status_t Status = hsa_init();

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

yaxunl added a comment. I tested the program on Windows and Linux and it works. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153725/new/ https://reviews.llvm.org/D153725 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.l

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

yaxunl updated this revision to Diff 536257. yaxunl marked 3 inline comments as done. yaxunl added a reviewer: arsenm. yaxunl added a comment. revised by comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153725/new/ https://reviews.llvm.org/D153725 Files: clang/tools/amdgpu-arch

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

yaxunl marked 3 inline comments as done. yaxunl added inline comments. Comment at: clang/tools/amdgpu-arch/AMDGPUArch.cpp:47 - // Attempt to load the HSA runtime. - if (llvm::Error Err = loadHSA()) { -logAllUnhandledErrors(std::move(Err), llvm::errs()); -return 1; -

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

yaxunl added a comment. In D153725#4463589 , @arsenm wrote: > Unrelated but can we get this to start reporting xnack and ecc? Using HIP runtime reports xnack and ecc. It reports the target ID including the accurate xnack and ecc feature which is ready t

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

jhuber6 added a comment. In D153725#4463589 , @arsenm wrote: > Unrelated but can we get this to start reporting xnack and ecc? A lot of CMake relies on this just being an ordered list of architectures, so we'd probably need to make that an opt-in thing.

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

arsenm added inline comments. Comment at: clang/tools/amdgpu-arch/AMDGPUArchByHIP.cpp:91 +} +printf("%s\n", prop.gcnArchName); + } llvm::outs CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153725/new/ https://reviews.llvm.org/D153725 _

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

arsenm added a comment. Unrelated but can we get this to start reporting xnack and ecc? Comment at: clang/tools/amdgpu-arch/AMDGPUArchByHIP.cpp:80 +if (err != hipSuccess) { + llvm::errs() << "Failed to get device id for ordinal " << i << "\n"; + return 1;

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

yaxunl created this revision. yaxunl added a reviewer: tra. Herald added subscribers: kerbowa, tpr, dstuttard, jvesely, kzhuravl. Herald added a project: All. yaxunl requested review of this revision. Herald added subscribers: jplehr, sstefan1, wdng. Herald added a reviewer: jdoerfert. Currently a