[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"))

[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

[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

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

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

[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

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

[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())) { Why does this handling need to be different between

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

jhuber6 added inline comments. Comment at: clang/test/OpenMP/amdgpu_target_with_aligned_attribute.c:1 +// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --function-signature --include-generated-funcs --replace-value-regex

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

jhuber6 added a comment. In D143306#4145482 , @jdoerfert wrote: > @jhuber6 Can you look into the last part, creating such a clang.cfg as part > of the build/install process? > > The fact that rpath is always set is a difference but I doubt it's that

[PATCH] D144505: [Clang] Add options in LTO mode when cross compiling for AMDGPU

jhuber6 added inline comments. Comment at: clang/test/Driver/amdgpu-toolchain.c:7 // RUN: %clang -### -g --target=amdgcn-mesa-mesa3d -mcpu=kaveri %s 2>&1 | FileCheck -check-prefix=DWARF_VER %s // AS_LINK: "-cc1as" arsenm wrote: > should add a test for

[PATCH] D144505: [Clang] Add options in LTO mode when cross compiling for AMDGPU

This revision was automatically updated to reflect the committed changes. Closed by commit rGc45d2df05e0e: [Clang] Add options in LTO mode when cross compiling for AMDGPU (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D144505: [Clang] Add options in LTO mode when cross compiling for AMDGPU

jhuber6 added inline comments. Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:545 +addLTOOptions(getToolChain(), Args, CmdArgs, Output, Inputs[0], + C.getDriver().getLTOMode() == LTOK_Thin); CmdArgs.push_back("-shared");

[PATCH] D144505: [Clang] Add options in LTO mode when cross compiling for AMDGPU

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

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

jhuber6 added a comment. If we're removing this feature it would probably make sense to remove the handling / flag altogether as `-fopenmp-implicit-rpath` isn't that much different from `-Wl,-rpath=/path/to/llvm/install/lib`. Overall, the problem is that we want to tie the `libomptarget.so`

[PATCH] D143768: [Clang] Add options to disable direct linking of arch tools

jhuber6 added a comment. Actually, I might want to optionally link this stuff in the `libc` project in the future as well. That would be a dependency only required for tests but it might be compelling then. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D143768: [Clang] Add options to disable direct linking of arch tools

jhuber6 added a comment. Like I said, the difference between these dependencies and something like `libxml2` or `libz` is that the presence of the libraries doesn't guard any features. We can build the application both ways, the only difference is runtime cost if we don't link the library

[PATCH] D143768: [Clang] Add options to disable direct linking of arch tools

jhuber6 added a comment. In D143768#4119021 , @aaronmondal wrote: > Does this address the case where we have HSA headers present on the system? > > Since this only seems to modify link time behavior, I think building with > this flag would still

[PATCH] D143768: [Clang] Add options to disable direct linking of arch tools

jhuber6 created this revision. jhuber6 added reviewers: aaronmondal, JonChesterfield, tra, jdoerfert, tianshilei1992. Herald added subscribers: kosarev, mattd, asavonic, kerbowa, tpr, jvesely. Herald added a project: All. jhuber6 requested review of this revision. Herald added a project: clang.

[PATCH] D143325: [Driver] Add -mllvm= as an alias for -mllvm

jhuber6 accepted this revision. jhuber6 added a comment. This revision is now accepted and ready to land. This looks good. I've had similar problems when trying to pass things with arguments via the `-Xarch` or `-Xopenmp-target=` options. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D142985: [LinkerWrapper] Fix memory issues due to unguarded accesses to global state

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9c4591d7f3ac: [LinkerWrapper] Fix memory issues due to unguarded accesses to global state (authored by jhuber6). Repository: rG LLVM Github

[PATCH] D142985: [LinkerWrapper] Fix memory issues due to unguarded accesses to global state

jhuber6 added a comment. In D142985#4095701 , @uabelho wrote: > I wrote > https://github.com/llvm/llvm-project/issues/60437 Great, I'll land it once the patch is accepted. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D142999: [Clang] Adjust PIC handling for the AMDGPU ToolChain

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9271c5da434b: [Clang] Adjust PIC handling for the AMDGPU ToolChain (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D142999: [Clang] Adjust PIC handling for the AMDGPU ToolChain

jhuber6 added inline comments. Comment at: clang/lib/Driver/ToolChains/AMDGPU.h:71 } - bool isPICDefaultForced() const override { return false; } + bool isPICDefaultForced() const override { return true; } bool SupportsProfiling() const override { return false; }

[PATCH] D142999: [Clang] Adjust PIC handling for the AMDGPU ToolChain

jhuber6 created this revision. jhuber6 added reviewers: arsenm, JonChesterfield, yaxunl, t-tye, msearles. 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] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

jhuber6 added a comment. In D141861#4094063 , @srj wrote: > Yes please! Let me know if this fixes anything rG9f64fbb882dc . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

jhuber6 added a comment. In D141861#4094058 , @srj wrote: > In D141861#4094043 , @jhuber6 wrote: > >> Would this just require checking `LLVM_BUILD_32_BITS`? Should be an easy >> change. > > I think so. (It might

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

jhuber6 added a comment. In D141861#4094036 , @srj wrote: > In D141861#4092237 , @tra wrote: > >> For what it's worth, NVIDIA has started deprecating 32-bit binaries long ago >>

[PATCH] D142985: [LinkerWrapper] Fix memory issues due to unguarded accesses to global state

jhuber6 added a comment. If this fixes the issues on your side, please open a bug so it can be backported. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142985/new/ https://reviews.llvm.org/D142985 ___

[PATCH] D142985: [LinkerWrapper] Fix memory issues due to unguarded accesses to global state

jhuber6 created this revision. jhuber6 added a reviewer: uabelho. Herald added a project: All. jhuber6 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. There were intemittent errors in the linker wrapper when using the sanitizers in

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

jhuber6 added a comment. In D141861#4092182 , @srj wrote: > In D141861#4092096 , @srj wrote: > >> Update: I may have a way to make this work from my side; testing now. > > Alas, that didn't work, stlll broken.

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

jhuber6 added a comment. In D141861#4091961 , @srj wrote: > In D141861#4091949 , @jhuber6 wrote: > >> In D141861#4091922 , @srj wrote: >> >>> Crosscompiling to x86-32 on

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

jhuber6 added a comment. In D141861#4091922 , @srj wrote: > Crosscompiling to x86-32 on an x86-64 host doesn't strike me as particularly > weird at all (especially on Windows), but apparently it is quite weird for > LLVM at this point in time as we

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

jhuber6 added a comment. In D141861#4091897 , @srj wrote: > It's finding a 64-bit CUDAToolkit, which it can't link against because the > rest of the build is 32-bit. Wondering why it didn't find it before then. But that's definitely a weird

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

jhuber6 added a comment. In D141861#4091851 , @srj wrote: >> https://github.com/llvm/llvm-project/commit/759dec253695f38a101c74905c819ea47392e515. >> Does it work if you revert this? I wouldn't think it wouldn't affect >> anything. That's the only

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

jhuber6 added a comment. In D141861#4091408 , @srj wrote: > In D141861#4091403 , @jhuber6 wrote: > >> In D141861#4091383 , @srj wrote: >> >>> It looks like this change

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

jhuber6 added a comment. In D141861#4091383 , @srj wrote: > It looks like this change (but not the rG4ce454c654bd > ) is in > the 17 branch, as the latter is now failing in the same

[PATCH] D141705: [HLSL] [Dirver] add dxv as a Driver Action Job

jhuber6 accepted this revision. jhuber6 added a comment. This revision is now accepted and ready to land. LGTM overall. Thanks for the patch. Others feel free to comment. Comment at: clang/lib/Driver/Driver.cpp:4214 } - + // Call validator for dxil when -Vd not in Args. +

[PATCH] D141705: [HLSL] [Dirver] add dxv as a Driver Action Job

jhuber6 added inline comments. Comment at: clang/lib/Driver/ToolChains/HLSL.cpp:170 + +Tool *clang::driver::toolchains::HLSLToolChain::getTool( +Action::ActionClass AC) const { python3kgae wrote: > jhuber6 wrote: > > I feel like this logic should go with the

[PATCH] D141705: [HLSL] [Dirver] add dxv as a Driver Action Job

jhuber6 added a comment. Thanks for the changes. Comment at: clang/lib/Driver/Driver.cpp:4226 +Args.ClaimAllArgs(options::OPT_cl_ignored_Group); + } nit. remember to `clang-format` Comment at: clang/lib/Driver/ToolChains/HLSL.cpp:170

[PATCH] D141705: [HLSL] [Dirver] add dxv as a Driver Action Job

jhuber6 added inline comments. Comment at: clang/lib/Driver/Driver.cpp:4217-4218 +// Only add action when needValidation. +const auto = getToolChain(Args, C.getDefaultToolChain().getTriple()); +const auto *HLSLTC = static_cast(); +if

[PATCH] D141705: [HLSL] [Dirver] add dxv as a Driver Action Job

jhuber6 added inline comments. Comment at: clang/test/Driver/dxc_dxv_path.hlsl:19-23 +// PHASES:+- 0: input, "[[INPUT:.+]]", hlsl +// PHASES:+- 1: preprocessor, {0}, c++-cpp-output +// PHASES:+- 2: compiler, {1}, ir +// PHASES:3: backend, {2}, assembler +// PHASES:4:

[PATCH] D141705: [HLSL] [Dirver] add dxv as a Driver Action Job

jhuber6 added inline comments. Comment at: clang/lib/Driver/Driver.cpp:4218 +// Only add action when needValidation. +if (toolchains::HLSLToolChain::needValidation(Args, *this, + C.getDefaultToolChain())) {

[PATCH] D141705: [HLSL] [Dirver] add dxv as a Driver Action Job

jhuber6 added inline comments. Comment at: clang/lib/Driver/Driver.cpp:4214-4216 + // Call validator for dxil when -Vd not in Args. + llvm::Triple T(getTargetTriple()); + if (T.getArch() == llvm::Triple::dxil) { Comment at:

[PATCH] D141705: [HLSL] [Dirver] add dxv as a Driver Action Job

jhuber6 added inline comments. Comment at: clang/lib/Driver/Driver.cpp:4216 + llvm::Triple T(getTargetTriple()); + if (T.getArch() == llvm::Triple::dxil && !Args.getLastArg(options::OPT_dxc_disable_validation)) { +// Only add action when 'dxv' exists.

[PATCH] D142650: [OpenMP] Run an extra 'OpenMPOpt' pass in LTO-mode

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6185246f4f62: [OpenMP] Run an extra OpenMPOpt pass in LTO-mode (authored by jhuber6). Changed prior to commit:

[PATCH] D142650: [OpenMP] Run an extra 'OpenMPOpt' pass in LTO-mode

jhuber6 updated this revision to Diff 492498. jhuber6 added a comment. Herald added subscribers: ormris, steven_wu. Fix test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142650/new/ https://reviews.llvm.org/D142650 Files:

[PATCH] D142650: [OpenMP] Run an extra 'OpenMPOpt' pass in LTO-mode

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, tianshilei1992, JonChesterfield, ronlieb, ye-luo, fhahn. Herald added subscribers: StephenFan, guansong, hiraditya, inglorion, yaxunl. Herald added a project: All. jhuber6 requested review of this revision. Herald added a

[PATCH] D141705: [HLSL] [Dirver] add dxv as a Driver Action Job

jhuber6 added a comment. I'm not overly familiar with HLSL or DirectX here. Most of the changes are purely mechanical, but I don't see anywhere we create the tool. Does that come later? Normally you'd test these with `-ccc-print-bindings`, `-ccc-print-bindings`, and `-###`.

[PATCH] D140158: [CUDA] Allow targeting NVPTX directly without a host toolchain

jhuber6 added a comment. In D140158#4082789 , @alexfh wrote: > This patch breaks our cuda compilations. The output file isn't created after > it: > > $ echo 'extern "C" __attribute__((global)) void q() {}' >q.cc > $ good-clang \ > -nocudainc

[PATCH] D142570: [nvptx-arch] Remove `find_package(CUDA)` as it has been deprecated.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG759dec253695: [nvptx-arch] Remove `find_package(CUDA)` as it has been deprecated. (authored by jhuber6). Repository: rG LLVM Github Monorepo

[PATCH] D142570: [nvptx-arch] Remove `find_package(CUDA)` as it has been deprecated.

jhuber6 created this revision. jhuber6 added reviewers: tra, jdoerfert, tianshilei1992, JonChesterfield. Herald added subscribers: mattd, gchakrabarti, asavonic, yaxunl. Herald added a project: All. jhuber6 requested review of this revision. Herald added a project: clang. Herald added a

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

jhuber6 added a comment. In D142484#4079869 , @Jake-Egan wrote: > Hi, this new test fails on AIX. Could you take a look? > https://lab.llvm.org/buildbot/#/builders/214/builds/5477/steps/6/logs/FAIL__Clang__linker-wrapper-libs_c I might just put that

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

jhuber6 added a comment. In D142484#4079869 , @Jake-Egan wrote: > Hi, this new test fails on AIX. Could you take a look? > https://lab.llvm.org/buildbot/#/builders/214/builds/5477/steps/6/logs/FAIL__Clang__linker-wrapper-libs_c I'm actually not sure why

[PATCH] D142491: [OpenMP] Do not link the bitcode OpenMP runtime when targeting AMDGPU.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5d1dc9fa043f: [OpenMP] Do not link the bitcode OpenMP runtime when targeting AMDGPU. (authored by jhuber6). Repository: rG LLVM Github Monorepo

[PATCH] D142486: [OpenMP] Unconditionally link the OpenMP device RTL static library

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGdc60f7aa0435: [OpenMP] Unconditionally link the OpenMP device RTL static library (authored by jhuber6). Repository: rG LLVM Github Monorepo

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

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 rG1964c334782e: [LinkerWrapper] Only import static libraries with needed symbols (authored by jhuber6).

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

jhuber6 marked an inline comment as done. jhuber6 added a comment. In D142484#4078377 , @tra wrote: > LGTM. Please wait a bit before landing it, in case @MaskRay has something to > say. I'm somewhat hoping to get this in before the fork that happens in

[PATCH] D142491: [OpenMP] Do not link the bitcode OpenMP runtime when targeting AMDGPU.

jhuber6 added a comment. In D142491#4078400 , @arsenm wrote: > Well the library still doesn't 100% work unless using -mlink-builtin-bitcode. > It's taking forever to make progress on fixing that This is the OpenMP DeviceRTL, the AMD one is still shoved

[PATCH] D141717: [Clang] Only emit textual LLVM-IR in device only mode

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd50dacd7c3c2: [Clang] Only emit textual LLVM-IR in device only mode (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

jhuber6 updated this revision to Diff 491883. jhuber6 added a comment. Adding test and making the logic a bit more readable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142484/new/ https://reviews.llvm.org/D142484 Files:

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

jhuber6 marked an inline comment as done. jhuber6 added a comment. In D142484#4077979 , @tra wrote: > We could also use more test cases. E.g. weak symbols (should not cause object > extraction) Yeah, I'll try to add a reasonable test here. It's a

[PATCH] D141717: [Clang] Only emit textual LLVM-IR in device only mode

jhuber6 updated this revision to Diff 491866. jhuber6 added a comment. Exempting HIP using the old driver. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141717/new/ https://reviews.llvm.org/D141717 Files: clang/lib/Driver/Driver.cpp

[PATCH] D141717: [Clang] Only emit textual LLVM-IR in device only mode

jhuber6 added a comment. In D141717#4074842 , @yaxunl wrote: > Can we keep the original behaviour for the old driver for HIP? Only enable > the change for the new driver. That's probably fair because AFAIK that will still use the

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

jhuber6 added inline comments. Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:1217-1218 +/// 1) It defines an undefined symbol in a regular object filie. +/// 2) It defines a global symbol without hidden visibility that has not +/// yet been

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

jhuber6 added a comment. In D142484#4077811 , @tra wrote: > @MaskRay - we seem to be reinventing the linker here and could use your > expertise. Yeah, this reinvents a lot of logic. But I don't think there's an easy way to get around this without

[PATCH] D142491: [OpenMP] Do not link the bitcode OpenMP runtime when targeting AMDGPU.

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, tianshilei1992, JonChesterfield, ronlieb, gregrodgers, jplehr. Herald added subscribers: kosarev, kerbowa, guansong, tpr, dstuttard, yaxunl, jvesely, kzhuravl. Herald added a project: All. jhuber6 requested review of this

[PATCH] D142486: [OpenMP] Unconditionally link the OpenMP device RTL static library

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, tianshilei1992, JonChesterfield. Herald added subscribers: guansong, yaxunl. Herald added a project: All. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits, sstefan1, MaskRay. Herald added a project:

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, tianshilei1992, JonChesterfield, tra, yaxunl. Herald added a subscriber: kosarev. Herald added a project: All. jhuber6 requested review of this revision. Herald added subscribers: openmp-commits, cfe-commits, sstefan1. Herald

[PATCH] D142393: [OpenMP] Add 'amdgpu-flat-work-group-size' to OpenMP kernels

jhuber6 added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:9552 F->addFnAttr("uniform-work-group-size", "true"); + if (IsOpenMPkernel) +F->addFnAttr("amdgpu-flat-work-group-size", arsenm wrote: > jhuber6 wrote: > > arsenm wrote: > > >

[PATCH] D142393: [OpenMP] Add 'amdgpu-flat-work-group-size' to OpenMP kernels

jhuber6 added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:9552 F->addFnAttr("uniform-work-group-size", "true"); + if (IsOpenMPkernel) +F->addFnAttr("amdgpu-flat-work-group-size", arsenm wrote: > Probably shouldn’t check the language,

[PATCH] D142393: [OpenMP] Add 'amdgpu-flat-work-group-size' to OpenMP kernels

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

[PATCH] D141717: [Clang] Only emit textual LLVM-IR in device only mode

jhuber6 added a comment. Ping, is it possible to resolve this before the fork tomorrow? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141717/new/ https://reviews.llvm.org/D141717 ___ cfe-commits mailing

[PATCH] D142138: [OpenMP] Clean up AMD handling for `-fopenmp-targets=amdgcn` arch inference

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG255922be7f00: [OpenMP] Clean up AMD handling for `-fopenmp-targets=amdgcn` arch inference (authored by jhuber6). Repository: rG LLVM Github

[PATCH] D133539: [OpenMP] Replace OpenMP register requires constructor with a global array

jhuber6 marked 2 inline comments as done. jhuber6 added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:10515 +default: + RequiresFlags.push_back(OMP_REQ_NONE); +} jdoerfert wrote: > Really? Not an error or unexpected state?

[PATCH] D142133: [LinkerWrapper] Use `clang` to perform the device linking

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGbec49b1d803c: [LinkerWrapper] Use `clang` to perform the device linking (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D142138: [OpenMP] Clean up AMD handling for `-fopenmp-targets=amdgcn` arch inference

jhuber6 added a comment. @yaxunl Do you know why AMD uses `-mcpu` but we use `-march` here? Would there be any downside to aliasing them in the AMDGPU toolchain? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142138/new/

[PATCH] D142138: [OpenMP] Clean up AMD handling for `-fopenmp-targets=amdgcn` arch inference

jhuber6 created this revision. jhuber6 added reviewers: tra, yaxunl, tianshilei1992, JonChesterfield, jdoerfert. Herald added subscribers: kosarev, kerbowa, guansong, jvesely. Herald added a project: All. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits, sstefan1,

[PATCH] D142133: [LinkerWrapper] Use `clang` to perform the device linking

jhuber6 created this revision. jhuber6 added reviewers: tra, JonChesterfield, tianshilei1992, jdoerfert. Herald added a project: All. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang. Right now in the linker wrapper we

[PATCH] D142075: [Clang][OpenMP] Allow `f16` literal suffix when compiling OpenMP target offloading for NVPTX

jhuber6 accepted this revision. jhuber6 added a comment. This revision is now accepted and ready to land. Seems reasonable, maybe update the comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142075/new/ https://reviews.llvm.org/D142075

[PATCH] D140158: [CUDA] Allow targeting NVPTX directly without a host toolchain

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. jhuber6 marked 3 inline comments as done. Closed by commit rG0660397e6809: [CUDA] Allow targeting NVPTX directly without a host toolchain (authored by jhuber6).

[PATCH] D140158: [CUDA] Allow targeting NVPTX directly without a host toolchain

jhuber6 marked 3 inline comments as done. jhuber6 added a comment. In D140158#4063720 , @tra wrote: > LGTM with few minor nits and questions. > > In D140158#4063689 , @jhuber6 wrote: > >> Addressing some

[PATCH] D140158: [CUDA] Allow targeting NVPTX directly without a host toolchain

jhuber6 updated this revision to Diff 490317. jhuber6 added a comment. Addressing some comments. I don't know if there's a cleaner way to mess around with the `.cubin` nonsense. I liked symbolic links but that doesn't work on Windows. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D140158: [CUDA] Allow targeting NVPTX directly without a host toolchain

jhuber6 marked an inline comment as done. jhuber6 added inline comments. Comment at: clang/lib/Driver/ToolChains/Cuda.h:196-197 + + void AddCudaIncludeArgs(const llvm::opt::ArgList , llvm::opt::ArgStringList ) const override; tra

[PATCH] D140158: [CUDA] Allow targeting NVPTX directly without a host toolchain

jhuber6 added inline comments. Comment at: clang/lib/Driver/ToolChains/Cuda.cpp:448-450 + // If we are invoking `nvlink` internally we need to output a `.cubin` file. + // Checking if the output is a temporary is the cleanest way to determine + // this. Putting this logic in

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

jhuber6 added a comment. In D141861#4060028 , @srj wrote: > This change appears to have broken the build when crosscompiling to x86-32 on > a Linux x86-64 system; on the Halide buildbots, we now fail at link time with > > FAILED: bin/nvptx-arch > :

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

jhuber6 added a comment. In D141861#4060028 , @srj wrote: > This change appears to have broken the build when crosscompiling to x86-32 on > a Linux x86-64 system; on the Halide buildbots, we now fail at link time with > > FAILED: bin/nvptx-arch > :

[PATCH] D141935: [OpenMP] Make `-Xarch_host` and `-Xarch_device` work for OpenMP offloading

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGeec516a0954a: [OpenMP] Make `-Xarch_host` and `-Xarch_device` work for OpenMP offloading (authored by jhuber6). Repository: rG LLVM Github

[PATCH] D141935: [OpenMP] Make `-Xarch_host` and `-Xarch_device` for for OpenMP offloading

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, tianshilei1992, tra, yaxunl. Herald added a subscriber: guansong. Herald added a project: All. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits, sstefan1, MaskRay. Herald added a project: clang.

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9954516ffb10: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build (authored by jhuber6). Changed prior to commit:

[PATCH] D141859: [amdgpu-arch] Dynamically load the HSA runtime if not found during the build

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf6ace23172e5: [amdgpu-arch] Dynamically load the HSA runtime if not found during the build (authored by jhuber6). Changed prior to commit:

[PATCH] D141717: [Clang] Only emit textual LLVM-IR in device only mode

jhuber6 added a comment. In D141717#4056971 , @yaxunl wrote: > The intention of -emit-llvm -S is usually to get LLVM assembly for all > targets for inspection or modification. HIP emits a bundled LLVM assembly in > textual format in this case. Users

[PATCH] D141861: [nvptx-arch] Dynamically load the CUDA runtime if not found during the build

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, tianshilei1992, tra, yaxunl, JonChesterfield. Herald added subscribers: mattd, gchakrabarti, asavonic. Herald added a project: All. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits, jholewinski.

[PATCH] D141859: [amdgpu-arch] Dynamically load the HSA runtime if not found during the build

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, tianshilei1992, tra, yaxunl, JonChesterfield, gregrodgers. Herald added subscribers: kosarev, kerbowa, tpr, dstuttard, jvesely, kzhuravl. Herald added a project: All. jhuber6 requested review of this revision. Herald added

[PATCH] D140158: [CUDA] Allow targeting NVPTX directly without a host toolchain

jhuber6 updated this revision to Diff 489554. jhuber6 added a comment. Updating. Used a different method to determine if we need to use `.cubin` or `.o`. It's a little ugly but I don't think there's a better way to do it. Also I just realized that if this goes through I could probably heavily

[PATCH] D141717: [Clang] Only emit textual LLVM-IR in device only mode

jhuber6 added a comment. I made the phases always go to `Assemble` but it didn't make a difference. We still get the textual IR here without the exception I added. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141717/new/

[PATCH] D141717: [Clang] Only emit textual LLVM-IR in device only mode

jhuber6 added a comment. In D141717#4053164 , @tra wrote: >> So are you suggesting that we complete the whole pipeline? So -S -emit-llvm >> gives host IR, but the device will go all the way to object? > > That would match my expectations and would solve

[PATCH] D141717: [Clang] Only emit textual LLVM-IR in device only mode

jhuber6 added a comment. In D141717#4052986 , @tra wrote: > In D141717#4052824 , @jhuber6 wrote: > >> For `-E` we don't embed anything, > > That was just an exaggerated example of top-level options affecting >

[PATCH] D141723: [Clang] Remove `CLANG_OPENMP_NVPTX_DEFAULT_ARCH` CMake option.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd1f4bfd2a8b0: [Clang] Remove `CLANG_OPENMP_NVPTX_DEFAULT_ARCH` CMake option. (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES

<    1   2   3   4   5   6   7   8   9   10   >