[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

jhuber6 added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1230 + llvm::sys::path::append(P, "llvm-libc"); + CmdArgs.push_back("-c-isystem"); + CmdArgs.push_back(Args.MakeArgString(P)); sivachandra wrote: > tra wrote: > > siva

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

jhuber6 added a comment. In D146973#4225641 , @aaron.ballman wrote: >> This lets offloading languages such as OpenMP use the system string.h when >> compiling for the host and then the LLVM libc string.h when targeting the >> GPU. > > How do we avoid A

[PATCH] D146975: [NVPTX] Add __CUDA_ARCH__ macro to standalone NVPTX compilations

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. jhuber6 marked an inline comment as done. Closed by commit rGbed7005eb4d4: [NVPTX] Add __CUDA_ARCH__ macro to standalone NVPTX compilations (authored by jhuber6). Chan

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

jhuber6 added a comment. In D146973#4225983 , @jdoerfert wrote: > I said this before, many times: > > We don't want to have different host and device libraries that are > incompatible. > Effectively, what we really want, is the host environment to just w

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

jhuber6 added a comment. In D146973#4227114 , @aaron.ballman wrote: > Hmmm, I've had experience with SYCL as to how it goes when you have > difference between host and device; those kinds of bugs are incredibly hard > to track down. Pointer sizes being

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

jhuber6 added a comment. In D146973#4227433 , @aaron.ballman wrote: > I am not asking you to implement a library based off another implementation's > specification. I am relaying implementation experience with the design you've > chosen for your implem

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

jhuber6 added a comment. In D146973#4228070 , @tra wrote: > I'm OK with injecting the path *now* with an understanding that it's a > short-term "happens to work" way to move forward while we're working on a > better solution. So, the proposed path forw

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

jhuber6 updated this revision to Diff 509090. jhuber6 added a comment. Changing to use the `gpu-none-llvm` subfolder name that @sivachandra recommended. Also adding a `--sysroot` argument to show that this include path shows up first. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

jhuber6 added a comment. In D145815#4230780 , @jeanPerier wrote: > @agozillon, in the test added here (omp-frontend-forwarding.f90), I am seeing > failures in some patches windows pre-merge checks that I think are not > related to the patches. > Could

[PATCH] D144569: [Clang][OpenMP] Fix accessing of aligned arrays in offloaded target regions

jhuber6 added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:2274 + if (!IsByRef) { +if ((Ctx.getTargetInfo().getTriple().isAMDGCN()) || +(Ctx.getTargetInfo().getTriple().isNVPTX())) { doru1004 wrote: > jhuber6 wrote: > > Why does this ha

[PATCH] D144569: [Clang][OpenMP] Fix accessing of aligned arrays in offloaded target regions

jhuber6 added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:2274 + if (!IsByRef) { +if ((Ctx.getTargetInfo().getTriple().isAMDGCN()) || +(Ctx.getTargetInfo().getTriple().isNVPTX())) { doru1004 wrote: > jhuber6 wrote: > > doru1004 wrote:

[PATCH] D144873: [OpenMP] Ignore implicit casts on assertion for `use_device_ptr`

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, tianshilei1992, ABataev, ronlieb, carlo.bertolli. Herald added subscribers: guansong, yaxunl. Herald added a project: All. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald added a p

[PATCH] D136100: [clang-format] Do not parse certain characters in pragma directives

jhuber6 added a comment. I would prefer that this doesn't get reverted, see the summary for the awful results for OpenMP without this patch. A potential solution would be to parse the next token and only add the indent if it's `omp`. Comment at: clang/unittests/Format/FormatT

[PATCH] D144884: [clang-format] Only add pragma continuation indentation for 'omp' clauses

jhuber6 created this revision. jhuber6 added reviewers: owenpan, MyDeveloperDay, HazardyKnusperkeks, hans. Herald added a project: All. jhuber6 requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang. The

[PATCH] D144873: [OpenMP] Ignore implicit casts on assertion for `use_device_ptr`

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG853d4059135b: [OpenMP] Ignore implicit casts on assertion for `use_device_ptr` (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D144884: [clang-format] Only add pragma continuation indentation for 'omp' clauses

jhuber6 added a comment. In D144884#4155730 , @jdoerfert wrote: > I'm assuming they have tests? I could add a test for the comment case in the bug report. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1448

[PATCH] D144884: [clang-format] Only add pragma continuation indentation for 'omp' clauses

jhuber6 added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:1279 + if (State.Line->InPragmaDirective) { +FormatToken *PragmaType = State.Line->First->Next->Next; +if (PragmaType && PragmaType->TokenText.equals("omp")) HazardyKnus

[PATCH] D144884: [clang-format] Only add pragma continuation indentation for 'omp' clauses

jhuber6 updated this revision to Diff 500872. jhuber6 added a comment. Add test for case in https://github.com/llvm/llvm-project/issues/59473 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144884/new/ https://reviews.llvm.org/D144884 Files: clang

[PATCH] D144993: [OpenMP]Emit captured decls for target data if no devices were specified.

jhuber6 accepted this revision. jhuber6 added a comment. This revision is now accepted and ready to land. LG, thanks a lot for the quick fix. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:7294 + // Emit helper decls of the use_device_ptr/use_device_addr clauses. +

[PATCH] D144884: [clang-format] Only add pragma continuation indentation for 'omp' clauses

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG466b4327f8fc: [clang-format] Only add pragma continuation indentation for 'omp' clauses (authored by jhuber6). Repository: rG LLVM Github Monorepo

[PATCH] D144884: [clang-format] Only add pragma continuation indentation for 'omp' clauses

jhuber6 added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:1280 +FormatToken *PragmaType = State.Line->First->Next->Next; +if (PragmaType && PragmaType->TokenText.equals("omp")) + return CurrentState.Indent + Style.ContinuationIndentWidth; -

[PATCH] D144884: [clang-format] Only add pragma continuation indentation for 'omp' clauses

jhuber6 added a comment. In D144884#4165192 , @MyDeveloperDay wrote: > without this change what does this look like? > > EXPECT_EQ( > "#pragma omp target \\\n" > "reduction(+ : var) \\\n" > "map(to : A[0 : N]) \\\n"

[PATCH] D129507: [OffloadPackager] Add option to extract files from images

jhuber6 added inline comments. Comment at: clang/test/Driver/offload-packager.c:2-3 +// REQUIRES: x86-registered-target +// REQUIRES: nvptx-registered-target +// REQUIRES: amdgpu-registered-target +// UNSUPPORTED: system-windows bader wrote: > Are nvptx and amdgp

[PATCH] D145591: [clang][HIP][OpenMP] Add warning if mixed HIP / OpenMP offloading

jhuber6 added a comment. I'm not a fan of the same warning being copied in 24 places. Why do we set `LangOpts.IsOpenMP` on the GPU compilation side, couldn't we just filter out the `-fopenmp` or whatever it is for the HIP job? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D145591: [clang][HIP][OpenMP] Add warning if mixed HIP / OpenMP offloading

jhuber6 added a comment. In D145591#4182748 , @yaxunl wrote: > In D145591#4182360 , @jhuber6 wrote: > >> I'm not a fan of the same warning being copied in 24 places. Why do we set >> `LangOpts.IsOpenMP` on the GP

[PATCH] D145820: Insert alloca for kernel args at function entry block instead of the launch point.

jhuber6 added a comment. No tests updated? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145820/new/ https://reviews.llvm.org/D145820 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists

[PATCH] D145941: [Clang] Always use -zdefs when linking AMDGPU images

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, JonChesterfield, arsenm, yaxunl, MaskRay. Herald added subscribers: kosarev, kerbowa, tpr, dstuttard, jvesely, kzhuravl. Herald added a project: All. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits,

[PATCH] D145944: [Clang] Add --version and --help messages to amdgpu/nvptx-arch

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, tianshilei1992, JonChesterfield, ye-luo. Herald added subscribers: kosarev, mattd, gchakrabarti, asavonic, kerbowa, tpr, dstuttard, yaxunl, jvesely, kzhuravl. Herald added a project: All. jhuber6 requested review of this revision.

[PATCH] D145944: [Clang] Add --version and --help messages to amdgpu/nvptx-arch

jhuber6 updated this revision to Diff 504670. jhuber6 added a comment. Add help print. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145944/new/ https://reviews.llvm.org/D145944 Files: clang/tools/amdgpu-arch/AMDGPUArch.cpp clang/tools/amdgpu-

[PATCH] D145941: [Clang] Always use -zdefs when linking AMDGPU images

jhuber6 added a comment. This can be turned off with `-zundefs`. So we could instruct people to use `-Wl,-zundefs` or `-Xoffload-linker -zundefs` if the old behavior is desired. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145941/new/ https://rev

[PATCH] D145944: [Clang] Add --version and --help messages to amdgpu/nvptx-arch

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa26aabefe535: [Clang] Add --version and --help messages to amdgpu/nvptx-arch (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D145862: [LinkerWrapper] Switch to add_clang_tool() macro

This revision was automatically updated to reflect the committed changes. Closed by commit rGc2aabcfc8395: [LinkerWrapper] Switch to add_clang_tool() macro (authored by foutrelis, committed by jhuber6). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM G

[PATCH] D145941: [Clang] Always use -zdefs when linking AMDGPU images

jhuber6 updated this revision to Diff 504818. jhuber6 added a comment. Use `--no-undefined` to be consistent with HIP and check for OpenCL. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145941/new/ https://reviews.llvm.org/D145941 Files: clang/l

[PATCH] D152391: [Clang] Allow bitcode linking when the input is LLVM-IR

jhuber6 added a subscriber: ronlieb. jhuber6 added a comment. ping @ronlieb Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152391/new/ https://reviews.llvm.org/D152391 ___ cfe-commits mailing list cfe-com

[PATCH] D152882: [LinkerWrapper] Support device binaries in multiple link jobs

jhuber6 updated this revision to Diff 532748. jhuber6 added a comment. I'm not sure why this keeps failing on Windows and have no clue how to tell what's going wrong. The builder simply says c:\ws\w4\llvm-project\premerge-checks\build\bin\clang-linker-wrapper.exe: error: invalid argument But

[PATCH] D152391: [Clang] Allow bitcode linking when the input is LLVM-IR

This revision was automatically updated to reflect the committed changes. Closed by commit rG8784b6a8540f: [Clang] Allow bitcode linking when the input is LLVM-IR (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152391/new/ https

[PATCH] D152442: [LinkerWrapper] Support linking vendor bitcode late

This revision was automatically updated to reflect the committed changes. Closed by commit rGefacdfc235e3: [LinkerWrapper] Support linking vendor bitcode late (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152442/new/ https://r

[PATCH] D153369: [OpenMP] Always apply target declarations to canonical definitions

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, tianshilei1992, JonChesterfield, tra, ABataev, carlo.bertolli. Herald added subscribers: sunshaoce, guansong. Herald added a project: All. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits, jplehr, ss

[PATCH] D153369: [OpenMP] Always apply target declarations to canonical definitions

jhuber6 updated this revision to Diff 533000. jhuber6 added a comment. Adding AST mutation listener to the other modified declaration to signal that it was changed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153369/new/ https://reviews.llvm.org

[PATCH] D153369: [OpenMP] Always apply target declarations to canonical definitions

jhuber6 updated this revision to Diff 533002. jhuber6 added a comment. Fix logic Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153369/new/ https://reviews.llvm.org/D153369 Files: clang/lib/Sema/SemaOpenMP.cpp clang/test/AST/dump.cpp Index: c

[PATCH] D153568: [ClangPackager] Add an option to extract inputs to an archive

jhuber6 created this revision. jhuber6 added reviewers: JonChesterfield, tra, yaxunl, jdoerfert, tianshilei1992, ronlieb, gregrodgers. Herald added a project: All. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits, jplehr, sstefan1. Herald added a project: clang. Cu

[PATCH] D153578: [Clang] Disable `libc` headers for offloading languages

jhuber6 created this revision. jhuber6 added reviewers: JonChesterfield, jdoerfert, tra, yaxunl. Herald added a subscriber: Anastasia. Herald added a project: All. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits, jplehr, sstefan1, MaskRay. Herald added a project: cl

[PATCH] D153578: [Clang] Disable `libc` headers for offloading languages

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG767852b1a8e3: [Clang] Disable `libc` headers for offloading languages (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D153568: [ClangPackager] Add an option to extract inputs to an archive

jhuber6 added a comment. In D153568#4445133 , @JonChesterfield wrote: > LGTM. As a meta level comment, we have far too many of these binary munging > sorts of tools. Definitely agree. This is supposed to be a direct repalcement for `clang-offload-bund

[PATCH] D153568: [ClangPackager] Add an option to extract inputs to an archive

This revision was automatically updated to reflect the committed changes. Closed by commit rG869baa912573: [ClangPackager] Add an option to extract inputs to an archive (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153568/new/

[PATCH] D153369: [OpenMP] Always apply target declarations to canonical definitions

jhuber6 added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153369/new/ https://reviews.llvm.org/D153369 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

[PATCH] D153667: [HIP]: Add gpu-link-output to control link job creation

jhuber6 added a comment. What's the difference here between this and the existing `--hip-link`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153667/new/ https://reviews.llvm.org/D153667 ___ cfe-commits

[PATCH] D153667: [HIP]: Add gpu-link-output to control link job creation

jhuber6 added a comment. In D153667#4450705 , @jrbyrnes wrote: > In D153667#4450517 , @jhuber6 wrote: > >> What's the difference here between this and the existing `--hip-link`? > > Hi @jhuber6 > > The commit is p

[PATCH] D153369: [OpenMP] Always apply target declarations to canonical definitions

jhuber6 added a comment. In D153369#4451993 , @ABataev wrote: > Did you try instead fix the OMPDeclareTargetDeclAttr::getActiveAttr() > function to make it look through all the declarations and return the > attribute from the first found instead of addi

[PATCH] D153369: [OpenMP] Always apply target declarations to canonical definitions

jhuber6 updated this revision to Diff 534950. jhuber6 added a comment. Updating to use `VD->redecls()`. Thanks for pointing that out, couldn't find it when I looked initially. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153369/new/ https://revie

[PATCH] D153369: [OpenMP] Always apply target declarations to canonical definitions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1d699bf2664d: [OpenMP] Always apply target declarations to canonical definitions (authored by jhuber6). Changed prior to commit: https://reviews.l

[PATCH] D153883: [Clang][OpenMP] Enable use of __kmpc_alloc_shared for VLAs defined in AMD GPU offloaded regions

jhuber6 added a comment. So this is implementing the `stacksave` using `__kmpc_alloc_shared` instead? It makes sense since the OpenMP standard expects sharing for the stack. I wonder how this interfaces with `-fopenmp-cuda-mode`. Comment at: clang/lib/CodeGen/CGDecl.cpp:1603

[PATCH] D153883: [Clang][OpenMP] Enable use of __kmpc_alloc_shared for VLAs defined in AMD GPU offloaded regions

jhuber6 added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:1085 } - for (const auto *VD : I->getSecond().EscapedVariableLengthDecls) { -// Use actual memory size of the VLA object including the padding doru1004 wrote: > ABataev wr

[PATCH] D153909: [AMDGPU] Always pass `-mcpu` to the `lld` linker

jhuber6 created this revision. jhuber6 added reviewers: JonChesterfield, yaxunl, tra. Herald added subscribers: kerbowa, tpr, dstuttard, jvesely, kzhuravl. Herald added a project: All. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, wdng. Herald added a pr

[PATCH] D153909: [AMDGPU] Always pass `-mcpu` to the `lld` linker

jhuber6 added a comment. In D153909#4454383 , @JonChesterfield wrote: > I thought lld did the right thing if you pass it raw IR, without specifying > an arch, but on reflection it might just be silently miscompiling things. The > test doesn't seem to i

[PATCH] D153667: [HIP]: Add gpu-link-output to control link job creation

jhuber6 added a comment. In D153667#4455943 , @yaxunl wrote: > For -fno-gpu-rdc case we do not use lto. Since -fno-gpu-rdc has one TU only, > we use the non-lto backend to get relocatable object, and use lld for > relocatable to shared object. This patc

[PATCH] D153909: [AMDGPU] Always pass `-mcpu` to the `lld` linker

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG765301183f58: [AMDGPU] Always pass `-mcpu` to the `lld` linker (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D154036: [libc] Add support for creating wrapper headers for offloading in clang

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, tianshilei1992, sivachandra, lntue, michaelrj, tra, JonChesterfield. Herald added projects: libc-project, All. Herald added a subscriber: libc-commits. jhuber6 requested review of this revision. Herald added subscribers: cfe-commit

[PATCH] D154036: [libc] Add support for creating wrapper headers for offloading in clang

jhuber6 added a comment. For reference, here is what one of the newly generated headers looks like that is used. #ifndef __LLVM_LIBC_DECLARATIONS_STDIO_H #define __LLVM_LIBC_DECLARATIONS_STDIO_H #ifndef __LIBC_ATTRS #define __LIBC_ATTRS #endif #ifdef __cplusplus extern "C" {

[PATCH] D154036: [libc] Add support for creating wrapper headers for offloading in clang

jhuber6 updated this revision to Diff 535600. jhuber6 added a comment. Hack around the `string` problem. GNU likes to provide different prototypes for C++. Manually disable this for now. Unsure if this will have reasonable fallout, but it seems bizarre that `string.h` would define C++ constructs

[PATCH] D154036: [libc] Add support for creating wrapper headers for offloading in clang

jhuber6 updated this revision to Diff 535835. jhuber6 added a comment. Semi-fix hack for `string.h` and fix `ctype.h`. `string.h` required undefining C++ mode so we didn't use weird GNU C++ handling, which we then still need the `extern "C"` for. The cytpe problems come from GNU defining everythin

[PATCH] D154145: [HIP] Fix -mllvm option for device lld linker

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

[PATCH] D154036: [libc] Add support for creating wrapper headers for offloading in clang

jhuber6 updated this revision to Diff 535983. jhuber6 added a comment. Add some checks to `stdlib.h` to ensure ABI compatibility for `div` functions. Because this patch makes us always include the LLVM libc repo when offloading, we mask off the wrappers if they were not installed by `libc`, so th

[PATCH] D154145: [HIP] Fix -mllvm option for device lld linker

jhuber6 added inline comments. Comment at: clang/lib/Driver/ToolChains/HIPAMD.cpp:164 +StringRef ArgVal = Arg->getValue(1); +if (ArgVal.startswith("-mllvm=")) { + ArgVal = ArgVal.substr(strlen("-mllvm=")); arsenm wrote: > StringRef Prefix("-mllvm=")

[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

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 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

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] D147365: [HIPSPV] Remove useIntegratedAs. NFC

jhuber6 added a comment. I don't know the toolchain, does `HIPSPV` perform as expected if you pass `-fno-integrated-as`? The difference is that `useIntegratedAs` forces it to always be enabled so the user can't change it AFAIK. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

jhuber6 added a comment. ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146973/new/ https://reviews.llvm.org/D146973 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

jhuber6 added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1196-1197 + // If we are compiling for a GPU target we want to override the system headers + // with ones created by the 'libc' project if present. + if (!Args.hasArg(options::OPT_nostdinc) &&

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf263bd8f7d4c: [Clang] Implicitly include LLVM libc headers for the GPU (authored by jhuber6). Changed prior to commit: https://reviews.llvm.org/D1

[PATCH] D102107: [OpenMP] Codegen aggregate for outlined function captures

jhuber6 updated this revision to Diff 510797. jhuber6 added a comment. Herald added subscribers: kbarton, nemanjai. Fixed the Clang tests. Haven't touched the LLVM ones because this breaks SPMDzation and state machine rewrites completely in those tests. Someone who knows what this patch changes

[PATCH] D147579: [nvptx-arch] Dynamically load `libcuda.so.1` directly instead

jhuber6 created this revision. jhuber6 added a reviewer: tra. Herald added subscribers: mattd, gchakrabarti, asavonic, yaxunl. Herald added a project: All. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits, jholewinski. Herald added a project: clang. This patch loads

[PATCH] D147579: [nvptx-arch] Dynamically load `libcuda.so.1` directly instead

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGad6a7d7dc9a2: [nvptx-arch] Dynamically load `libcuda.so.1` directly instead (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D147666: [OPENMP] Adds /lib to rpath to avoid need to set LD_LIBRARY_PATH to find plugins.

jhuber6 added a comment. Using `-rpath` by default with OpenMP was removed after a long conversation in https://reviews.llvm.org/D143306. The way forward is most likely to have AOMP provide this in a resource file configuration. I think @ronlieb has a working version of that. Repository: rG

[PATCH] D147572: [Clang][OpenMP] Fix failure with team-wide allocated variable

jhuber6 added inline comments. Comment at: clang/test/OpenMP/target_team_variable_codegen.cpp:33 +//. +// CHECK-NVIDIA: @local_a = internal addrspace(3) global [10 x i32] zeroinitializer, align 4 +//. Shouldn't the Nvidia version also be undefined? Not sure why

[PATCH] D147572: [Clang][OpenMP] Fix failure with team-wide allocated variable

jhuber6 added inline comments. Comment at: clang/test/OpenMP/target_team_variable_codegen.cpp:33 +//. +// CHECK-NVIDIA: @local_a = internal addrspace(3) global [10 x i32] zeroinitializer, align 4 +//. jdoerfert wrote: > doru1004 wrote: > > jhuber6 wrote: > > > S

[PATCH] D148444: [clang-tidy] Prevent `llvmlibc-inline-function-decl` triggering on lambdas

jhuber6 created this revision. jhuber6 added reviewers: lntue, michaelrj, sivachandra, gchatelet, goldstein.w.n. Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun. Herald added a reviewer: njames93. Herald added a project: All. jhuber6 requested review of this revision. Herald added a p

[PATCH] D148444: [clang-tidy] Prevent `llvmlibc-inline-function-decl` triggering on lambdas

jhuber6 updated this revision to Diff 513990. jhuber6 added a comment. Add test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148444/new/ https://reviews.llvm.org/D148444 Files: clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp

[PATCH] D148444: [clang-tidy] Prevent `llvmlibc-inline-function-decl` triggering on lambdas

jhuber6 updated this revision to Diff 513997. jhuber6 added a comment. Address nit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148444/new/ https://reviews.llvm.org/D148444 Files: clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cp

[PATCH] D148444: [clang-tidy] Prevent `llvmlibc-inline-function-decl` triggering on lambdas

jhuber6 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp:64-67 +// CHECK-MESSAGES-NOT: :[[@LINE+4]]:3: warning: '__invoke' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of th

[PATCH] D148444: [clang-tidy] Prevent `llvmlibc-inline-function-decl` triggering on lambdas

jhuber6 added a comment. In D148444#4272036 , @PiotrZSL wrote: > Fix Linux build before committing & resolve all comments. The log says that it failed because of the CMake version. I don't think I can fix that. Comment at: clang-too

[PATCH] D148444: [clang-tidy] Prevent `llvmlibc-inline-function-decl` triggering on lambdas

jhuber6 updated this revision to Diff 514001. jhuber6 added a comment. Rebasing on main Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148444/new/ https://reviews.llvm.org/D148444 Files: clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclChe

[PATCH] D148444: [clang-tidy] Prevent `llvmlibc-inline-function-decl` triggering on lambdas

This revision was automatically updated to reflect the committed changes. Closed by commit rG1663016b41d7: [clang-tidy] Prevent `llvmlibc-inline-function-decl` triggering on lambdas (authored by jhuber6). Changed prior to commit: https://reviews.llvm.org/D148444?vs=514001&id=514026#toc Reposit

[PATCH] D147572: [Clang][OpenMP] Fix failure with team-wide allocated variable

jhuber6 accepted this revision. jhuber6 added a comment. This revision is now accepted and ready to land. LGTM unless anyone else has any concerns. Comment at: clang/test/OpenMP/target_team_variable_codegen.cpp:33 +//. +// CHECK-NVIDIA: @local_a = internal addrspace(3) global [

[PATCH] D150930: [Driver] Accept and ignore -fno-lifetime-dse argument

jhuber6 added a comment. In D150930#4355240 , @MaskRay wrote: > Note: for options controlling individual optimization behaviors, there is a > large probability that they may not make sense for Clang since the two > compilers' internals are so different.

[PATCH] D150998: [OpenMP] Fix using the target ID when using the new driver

jhuber6 created this revision. jhuber6 added reviewers: JonChesterfield, carlo.bertolli, yaxunl, saiislam. Herald added subscribers: sunshaoce, kerbowa, guansong, tpr, jvesely. Herald added a project: All. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits, jplehr, sst

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

jhuber6 added a comment. Herald added a subscriber: arichardson. Herald added a project: All. Ran into this change trying to decay a qualifier pointer to a generic address space, e.g. https://godbolt.org/z/3dEd4TxjW. I understand that `addrspace_cast` was added to replace this functionality, but

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

jhuber6 added a comment. In D58346#4359631 , @ebevhan wrote: > In D58346#4359291 , @jhuber6 wrote: > >> should C++ really be limited by OpenCL here? > > It probably shouldn't. There's many places in the codebase whe

[PATCH] D151087: [Clang] Permit address space casts with 'reinterpret_cast' in C++

jhuber6 created this revision. jhuber6 added reviewers: rjmccall, ebevhan, jdoerfert, JonChesterfield, aaron.ballman. Herald added subscribers: arichardson, Anastasia. Herald added a project: All. jhuber6 requested review of this revision. Herald added a project: clang. Herald added a subscriber:

[PATCH] D151087: [Clang] Permit address space casts with 'reinterpret_cast' in C++

jhuber6 added a comment. In D151087#4360577 , @ebevhan wrote: > What would be the semantics of such an operation if the address spaces are > disjoint? Or, if the underlying pointer widths aren't the same? It would most likely invalid, but I'm not assert

[PATCH] D151098: [Clang][Docs] Add help test to `-march` and `-mcpu` to suggest `-mcpu=help`

jhuber6 created this revision. jhuber6 added reviewers: MaskRay, aaron.ballman. Herald added a project: All. jhuber6 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Currently there is no documentation for these flags, users might find it co

[PATCH] D151087: [Clang] Permit address space casts with 'reinterpret_cast' in C++

jhuber6 added a comment. In D151087#4360668 , @arsenm wrote: >> It would most likely invalid, but I'm not asserting that `clang` should be >> responsible for diagnosing misuse in these cases. Especially because in >> generic freestanding C++ we don't ha

[PATCH] D150998: [OpenMP] Fix using the target ID when using the new driver

jhuber6 added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:8465-8470 +if (TC->getTriple().isAMDGPU()) { + for (StringRef Feature : llvm::split(Arch.split(':').second, ':')) { +FeatureArgs.emplace_back( +Args.MakeArgString(Feature

[PATCH] D151087: [Clang] Permit address space casts with 'reinterpret_cast' in C++

jhuber6 added a comment. In D151087#4360729 , @ebevhan wrote: > That's fair. I would like clang to improve and formalize the semantics for > generic address space behavior a bit, which was part of the point with D62574 >

[PATCH] D151098: [Clang][Docs] Add help test to `-march` and `-mcpu` to suggest `-mcpu=help`

jhuber6 added a comment. In D151098#4360750 , @aaron.ballman wrote: > LGTM, though it's a bit jarring that `-march` suggests using `-mcpu=help` > (should we ensure that `-march=help` works on all targets instead?) Using `-march=help` works inadvertentl

[PATCH] D151087: [Clang] Permit address space casts with 'reinterpret_cast' in C++

jhuber6 added a comment. In D151087#4360818 , @ebevhan wrote: > By "freestanding C++" I assume you mean "C++ without the OpenCL C++ > extension" and not "C++ without extensions at all", because in the latter > case, you don't have address spaces either.

[PATCH] D151087: [Clang] Permit address space casts with 'reinterpret_cast' in C++

jhuber6 added a comment. In D151087#4360919 , @arsenm wrote: > In D151087#4360695 , @jhuber6 wrote: > >> I don't think that's something we can diagnose here with just the address >> space number. it would require

<    7   8   9   10   11   12   13   14   15   16   >