[PATCH] D128090: [Clang][OpenMP] Process multi-arch compilation options given via -march

jhuber6 added a comment. In D128090#3649202 , @tra wrote: > In D128090#3649125 , @jhuber6 wrote: > >> It just defaults to `sm_35` if CUDA isn't present on the system IIRC. >> Alternatively we could ship a tool to

[PATCH] D129655: [CUDA] Allow the new driver to compile CUDA in non-RDC mode

jhuber6 added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6998-6999 +CmdArgs.push_back(CudaDeviceInput->getFilename()); +if (IsRDCMode) + CmdArgs.push_back("-fgpu-rdc"); + } else if (IsCuda && !HostOffloadingInputs.empty() && !IsRDCMode) { ---

[PATCH] D129655: [CUDA] Allow the new driver to compile CUDA in non-RDC mode

jhuber6 marked an inline comment as done. jhuber6 added a comment. It's also worth noting that this doesn't include the `PTX` output for JIT in the fatbinary, it would be relatively easy to include that but I wanted to ask how we should handle that. Comment at: clang/lib/Driv

[PATCH] D129655: [CUDA] Allow the new driver to compile CUDA in non-RDC mode

jhuber6 updated this revision to Diff 444398. jhuber6 marked an inline comment as done. jhuber6 added a comment. Updating and making suggested changes. I removed the old `fgpu-rdc` in rG6abaa8e2103760025cee76528f555de7cf6698e6

[PATCH] D129655: [CUDA] Allow the new driver to compile CUDA in non-RDC mode

jhuber6 marked an inline comment as done. jhuber6 added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7009 + // Host-side offloading compilation receives all device-side outputs. Include + // them in the host compilation depending on the target. if ((IsC

[PATCH] D128090: [Clang][OpenMP] Process multi-arch compilation options given via -march

jhuber6 added a comment. In D128090#3649579 , @tra wrote: > In D128090#3649235 , @jhuber6 wrote: > >> Interesting, may be worthwhile to query that if it exists, though AMD does >> this with `amdgpu-arch` which ha

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

jhuber6 added a comment. Thanks for the patch. I still think this is a silly feature to support, but users will probably expect it. See comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:10790 Flags = OffloadEntriesInfoManagerTy::OMPTargetGlobalVarEntryTo; -V

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

jhuber6 added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:3284-3286 // Hidden or internal symbols on the device are not externally visible. We // should not attempt to register them by creating an offloading entry. if (auto *GV = dyn_c

[PATCH] D129655: [CUDA] Allow the new driver to compile CUDA in non-RDC mode

This revision was automatically updated to reflect the committed changes. jhuber6 marked an inline comment as done. Closed by commit rGb370be37cca7: [CUDA] Allow the new driver to compile CUDA in non-RDC mode (authored by jhuber6). Changed prior to commit: https://reviews.llvm.org/D129655?vs=44

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

jhuber6 added inline comments. Comment at: clang/test/OpenMP/target_update_messages.cpp:17 -static int y; -#pragma omp declare target(y) - -void yyy() { -#pragma omp target update to(y) // expected-error {{the host cannot update a declare target variable that is not externally

[PATCH] D129581: [Clang] Rework LTO argument handling in the linker wrapper

jhuber6 added a comment. On second thought I'm not sure if overloading `-plugin-opt` is a good idea because we could have situations where we'd want them to be mutually exclusive, although I'd like to reuse the logic to set the arguments. I could change it to emit `-offload-opt=` instead or som

[PATCH] D129784: [HIP] Allow the new driver to compile HIP in non-RDC mode

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, tra, yaxunl, JonChesterfield. Herald added a project: All. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. The new driver primarily allows us to support RDC-

[PATCH] D129885: [CUDA] Make the new driver properly ignore non-CUDA inputs

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, tra, yaxunl. Herald added subscribers: mattd, carlosgalvezp. Herald added a project: All. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. The new driver gene

[PATCH] D129885: [CUDA] Make the new driver properly ignore non-CUDA inputs

jhuber6 updated this revision to Diff 445087. jhuber6 added a comment. Adjusting tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129885/new/ https://reviews.llvm.org/D129885 Files: clang/lib/Driver/Driver.cpp clang/test/Driver/cuda-phases.

[PATCH] D129885: [CUDA] Make the new driver properly ignore non-CUDA inputs

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGbb957a8d524c: [CUDA] Make the new driver properly ignore non-CUDA inputs (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D130096: [Clang][AMDGPU] Emit AMDGPU library control constants in clang

jhuber6 created this revision. jhuber6 added reviewers: JonChesterfield, yaxunl, saiislam, arsenm, carlo.bertolli, MaskRay, jdoerfert, tianshilei1992. Herald added subscribers: kosarev, StephenFan, t-tye, tpr, dstuttard, jvesely, kzhuravl. Herald added a project: All. jhuber6 requested review of

[PATCH] D130096: [Clang][AMDGPU] Emit AMDGPU library control constants in clang

jhuber6 added a comment. Let me know if I should move this code somewhere else, or if there are problems. One change I made is that the constant is `weak_odr` and `hidden` instead of `linkonce_odr` and `protected`. This is so this constant is alive until link time, AMDGPU pretty much always use

[PATCH] D130096: [Clang][AMDGPU] Emit AMDGPU library control constants in clang

jhuber6 added a comment. In D130096#3663010 , @JonChesterfield wrote: > Tagging Brian as the code owner of rocm device libs - emitting these in clang > would simplify that library. > > Currently clang reads these commandline flags and conditionally link

[PATCH] D130096: [Clang][AMDGPU] Emit AMDGPU library control constants in clang

jhuber6 added a comment. In D130096#3663062 , @JonChesterfield wrote: > A safer bet is to use the current control flow that links in specific bitcode > files, but create the global directly instead of linking in the file. That'll > give us zero semanti

[PATCH] D130096: [Clang][AMDGPU] Emit AMDGPU library control constants in clang

jhuber6 marked an inline comment as done. jhuber6 added a comment. In D130096#3663295 , @yaxunl wrote: > There is no constant propagation for globals with weak linage, right? > Otherwise, it won't work. My concern is that there may be optimization passes

[PATCH] D130096: [Clang][AMDGPU] Emit AMDGPU library control constants in clang

jhuber6 added a comment. In D130096#3663295 , @yaxunl wrote: > There is no constant propagation for globals with weak linage, right? > Otherwise, it won't work. My concern is that there may be optimization passes > which do not respect the weak linkage

[PATCH] D130096: [Clang][AMDGPU] Emit AMDGPU library control constants in clang

jhuber6 added a comment. In D130096#3663411 , @arsenm wrote: > In D130096#3663398 , @jhuber6 wrote: > >> In D130096#3663295 , @yaxunl wrote: >> >>> There is no constant pr

[PATCH] D130096: [Clang][AMDGPU] Emit AMDGPU library control constants in clang

jhuber6 added a comment. In D130096#3663062 , @JonChesterfield wrote: > A safer bet is to use the current control flow that links in specific bitcode > files, but create the global directly instead of linking in the file. That'll > give us zero semanti

[PATCH] D129784: [HIP] Allow the new driver to compile HIP in non-RDC mode

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

[PATCH] D130096: [Clang][AMDGPU] Emit AMDGPU library control constants in clang

jhuber6 added a comment. In D130096#3666155 , @yaxunl wrote: > The current patch does not consider HIP/OpenCL compile options, therefore the > value of these variables are not correct for OpenCL/HIP. They need to be > overridden by the variables with th

[PATCH] D129784: [HIP] Allow the new driver to compile HIP in non-RDC mode

This revision was automatically updated to reflect the committed changes. Closed by commit rG0c1b32717bcf: [HIP] Allow the new driver to compile HIP in non-RDC mode (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129784/new/ htt

[PATCH] D122683: [OpenMP] Use new offloading binary when embedding offloading images

jhuber6 added a comment. In D122683#3668412 , @mgorny wrote: > I'm sorry for noticing it this late but this change seems to have broken the > test on 32-bit x86: Seems to be a difference in the alignment, I'm not sure why this changes though because th

[PATCH] D122683: [OpenMP] Use new offloading binary when embedding offloading images

jhuber6 added a comment. In D122683#3668423 , @jhuber6 wrote: > In D122683#3668412 , @mgorny wrote: > >> I'm sorry for noticing it this late but this change seems to have broken the >> test on 32-bit x86: > > See

[PATCH] D127304: [LinkerWrapper] Embed OffloadBinaries for OpenMP offloading images

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG080022d8ed6c: [LinkerWrapper] Embed OffloadBinaries for OpenMP offloading images (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANG

[PATCH] D125256: [OpenMP] Add __CUDA_ARCH__ definition when offloading with OpenMP

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

[PATCH] D125265: [OpenMP] Don't include the device wrappers if -nostdinc is used

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

[PATCH] D123471: [CUDA] Create offloading entries when using the new driver

jhuber6 added inline comments. Comment at: clang/lib/CodeGen/CGCUDARuntime.h:58 + /// Mark the entry as a global variable. + OffloadGlobalEntry = 0x0, + /// Mark the entry as a managed global variable. tra wrote: > jhuber6 wrote: > > tra wrote: > >

[PATCH] D125265: [OpenMP] Don't include the device wrappers if -nostdinc is used

jhuber6 added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1311 if (JA.isDeviceOffloading(Action::OFK_OpenMP) && + !Args.hasArg(options::OPT_nostdlib) && (getToolChain().getTriple().isNVPTX() || tra wrote: > Did you mean `nost

[PATCH] D125265: [OpenMP] Don't include the device wrappers if -nostdinc is used

jhuber6 updated this revision to Diff 428181. jhuber6 added a comment. Fix using `nostdlib` instead of `nostdinc` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125265/new/ https://reviews.llvm.org/D125265 Files: clang/lib/Driver/ToolChains/Clang

[PATCH] D123471: [CUDA] Create offloading entries when using the new driver

jhuber6 updated this revision to Diff 428186. jhuber6 added a comment. Updating comments to desceibe usage of flags with zero size. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123471/new/ https://reviews.llvm.org/D123471 Files: clang/include/c

[PATCH] D123471: [CUDA] Create offloading entries when using the new driver

jhuber6 updated this revision to Diff 428188. jhuber6 added a comment. Clang format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123471/new/ https://reviews.llvm.org/D123471 Files: clang/include/clang/Basic/LangOptions.def clang/include/clang

[PATCH] D125165: [Clang] Introduce clang-offload-binary tool to bundle device files

jhuber6 added a comment. Thanks for the comments. Comment at: clang/docs/ClangOffloadBinary.rst:8 + +.. _clang-offload-binary: + tra wrote: > Naming nit: `binary` may not be the best term for what we're trying to do > here. Perhaps something like `package`, `c

[PATCH] D125314: [OpenMP] Don't set device runtime debugging flags if using '-nogpulib'

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, tianshilei1992, ABataev, 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 p

[PATCH] D125314: [OpenMP] Don't set device runtime debugging flags if using '-nogpulib'

jhuber6 added a comment. In D125314#3503453 , @jdoerfert wrote: > Not actually tested, is it? Should be, we already have a test for not emitting these when the host-or file path is empty, I just reused the same tests but set the flags to use `nogpulib`

[PATCH] D125165: [Clang] Introduce clang-offload-binary tool to bundle device files

jhuber6 updated this revision to Diff 428379. jhuber6 added a comment. Changing name from clang-offload-binary to clang-offload-packager and updating some help mesages. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125165/new/ https://reviews.llvm

[PATCH] D125165: [Clang] Introduce clang-offload-packager tool to bundle device files

jhuber6 added inline comments. Comment at: clang/docs/ClangOffloadBinary.rst:15 +metadata. We use a custom binary format for bundling all the device images +together. The image format is a small header wrapping around a string map. This +tool creates bundled binaries so that they

[PATCH] D125165: [Clang] Introduce clang-offload-packager tool to bundle device files

jhuber6 updated this revision to Diff 428443. jhuber6 added a comment. Adding some extra documentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125165/new/ https://reviews.llvm.org/D125165 Files: clang/docs/ClangOffloadPackager.rst clang

[PATCH] D125165: [Clang] Introduce clang-offload-packager tool to bundle device files

jhuber6 added a comment. In D125165#3504252 , @tra wrote: > LGTM in principle. > > Given that we're introducing a new tool dependency we may want to get a stamp > from someone dealing with build and release. > @tstellar -- do we need to change anything

[PATCH] D123471: [CUDA] Create offloading entries when using the new driver

jhuber6 added a comment. Is this with D123810 and D123812 good to land? It would be nice to be able to test this upstream. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123471/n

[PATCH] D125333: [OpenMP] Use the new OpenMP device static library when doing LTO

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

[PATCH] D123810: [Cuda] Add initial support for wrapping CUDA images in the new driver.

jhuber6 added inline comments. Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:726 + + if (Error Err = executeCommands(*FatBinaryPath, CmdArgs)) +return std::move(Err); tra wrote: > We should have a way to pass extra options to fatbinary,

[PATCH] D123810: [Cuda] Add initial support for wrapping CUDA images in the new driver.

jhuber6 updated this revision to Diff 428517. jhuber6 added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Addressing comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123810/new/ https://reviews.llvm.org/D12

[PATCH] D123810: [Cuda] Add initial support for wrapping CUDA images in the new driver.

jhuber6 added a comment. Thanks for the review. Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:193 + } + static unsigned getHashValue(const OffloadKind &Val) { return Val * 37U; } + tra wrote: > Is there a particular reason for multiplyin

[PATCH] D123812: [CUDA] Add wrapper code generation for registering CUDA images

jhuber6 added inline comments. Comment at: clang/tools/clang-linker-wrapper/OffloadWrapper.cpp:351 +/// required for texture / surface / managed variables. +Function *createRegisterGlobalsFunction(Module &M) { + LLVMContext &C = M.getContext(); tra wrote:

[PATCH] D123471: [CUDA] Create offloading entries when using the new driver

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0035f7154c2a: [CUDA] Create offloading entries when using the new driver (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D123810: [Cuda] Add initial support for wrapping CUDA images in the new driver.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe7858a9fab8c: [Cuda] Add initial support for wrapping CUDA images in the new driver. (authored by jhuber6). Repository: rG LLVM Github Monorepo C

[PATCH] D123812: [CUDA] Add wrapper code generation for registering CUDA images

This revision was automatically updated to reflect the committed changes. Closed by commit rGf49d576a882d: [CUDA] Add wrapper code generation for registering CUDA images (authored by jhuber6). Changed prior to commit: https://reviews.llvm.org/D123812?vs=426467&id=428622#toc Repository: rG LL

[PATCH] D125165: [Clang] Introduce clang-offload-packager tool to bundle device files

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG26eb04268f4c: [Clang] Introduce clang-offload-packager tool to bundle device files (authored by jhuber6). Herald added a subscriber: kosarev. Change

[PATCH] D125165: [Clang] Introduce clang-offload-packager tool to bundle device files

jhuber6 added a comment. In D125165#3506172 , @thakis wrote: > We now have clang-offload-bundler, clang-offload-packager, > clang-offload-wrapper. Do we really need that many distinct binaries for > offloading? Any chance we could combine some of those?

[PATCH] D125165: [Clang] Introduce clang-offload-packager tool to bundle device files

jhuber6 added a comment. In D125165#3506502 , @thakis wrote: > In D125165#3506477 , @yaxunl wrote: > >> In D125165#3506448 , @thakis wrote: >> >>> We could add a "clang-of

[PATCH] D125165: [Clang] Introduce clang-offload-packager tool to bundle device files

jhuber6 added inline comments. Comment at: clang/docs/ClangOffloadPackager.rst:31-32 +uint32_t Flags; +StringMap StringData; +MemoryBufferRef Image; + }; yaxunl wrote: > This makes the file format depend on LLVM version and potentially standard > C+

[PATCH] D125165: [Clang] Introduce clang-offload-packager tool to bundle device files

jhuber6 added inline comments. Comment at: clang/docs/ClangOffloadPackager.rst:31-32 +uint32_t Flags; +StringMap StringData; +MemoryBufferRef Image; + }; yaxunl wrote: > jhuber6 wrote: > > yaxunl wrote: > > > This makes the file format depend on LLVM

[PATCH] D125256: [OpenMP] Add `__CUDA_ARCH__` definition when offloading with OpenMP

This revision was automatically updated to reflect the committed changes. Closed by commit rG002a63f937d9: [OpenMP] Add `__CUDA_ARCH__` definition when offloading with OpenMP (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125256

[PATCH] D125265: [OpenMP] Don't include the device wrappers if -nostdinc is used

This revision was automatically updated to reflect the committed changes. Closed by commit rG5189f634a113: [OpenMP] Don't include the device wrappers if -nostdinc is used (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125265/new

[PATCH] D125314: [OpenMP] Don't set device runtime debugging flags if using '-nogpulib'

This revision was automatically updated to reflect the committed changes. Closed by commit rGaf757f89806e: [OpenMP] Don't set device runtime debugging flags if using '-nogpulib' (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125

[PATCH] D125333: [OpenMP] Use the new OpenMP device static library when doing LTO

This revision was automatically updated to reflect the committed changes. Closed by commit rG4638ae3a8575: [OpenMP] Use the new OpenMP device static library when doing LTO (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125333/ne

[PATCH] D125256: [OpenMP] Add `__CUDA_ARCH__` definition when offloading with OpenMP

jhuber6 added a comment. In D125256#3513596 , @RKSimon wrote: > @jhuber6 I think this or one of your other openmp commits has caused the > Driver/cuda-openmp-driver.cu test failure here: > https://lab.llvm.org/buildbot/#/builders/214/builds/1274/steps/6

[PATCH] D125705: [OpenMP] Don't build the offloading driver without a source input

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, tianshilei1992, wsmoses. 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: clang. T

[PATCH] D125705: [OpenMP] Don't build the offloading driver without a source input

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb653b409ff44: [OpenMP] Don't build the offloading driver without a source input (authored by jhuber6). Changed prior to commit: https://reviews.ll

[PATCH] D125705: [OpenMP] Don't build the offloading driver without a source input

jhuber6 added a comment. In D125705#3519570 , @yaxunl wrote: > HIP toolchain allows clang driver to compile bundled bitcode or assembly for > mixed host/device compilation or device-only multi-GPU compilation. > > e.g. > > clang --offload-arch=gfx906 --o

[PATCH] D125904: [Cuda] Use fallback method to mangle externalized decls if no CUID given

jhuber6 created this revision. jhuber6 added reviewers: yaxunl, tra. Herald added subscribers: mattd, carlosgalvezp. Herald added a project: All. jhuber6 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. CUDA requires that static variables be

[PATCH] D125904: [Cuda] Use fallback method to mangle externalized decls if no CUID given

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

[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

jhuber6 created this revision. jhuber6 added reviewers: markdewing, 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

[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

jhuber6 added a comment. In D126226#3532127 , @tra wrote: > `-Xoffload-linker= ` > > The syntax is confusing. Normally only `triple` would be the argument for > `-Xoffload-linker` option. > Having both `-Xoffload-linker` and `-Xoffload-linker=` variants

[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

jhuber6 added a comment. In D126226#3532216 , @tra wrote: > You do not need to hardcode it. The idea of `JoinedAndSeparate` is that an > option `foo` assepts two argumants, one glued to it and another following > after a whitespace. > So, when you defin

[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

jhuber6 updated this revision to Diff 431452. jhuber6 added a comment. Changing the `-Xoffload-linker=` to `-Xoffload-linker-`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126226/new/ https://reviews.llvm.org/D126226 Files: clang/include/clang

[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

jhuber6 added a comment. In D126226#3532257 , @MaskRay wrote: > It's better to avoid `JoinedAndSeparate` for new options. It is for `--xxx > val` and `--xxxval` but not intended for the option this patch will add. So how should I pass these two argument

[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

jhuber6 updated this revision to Diff 431475. jhuber6 added a comment. Updating to use @MaskRay's suggestion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126226/new/ https://reviews.llvm.org/D126226 Files: clang/include/clang/Driver/Options.td

[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

jhuber6 added a comment. In D126226#3532423 , @tra wrote: > We keep running into the same old underlying issue that we do not have a good > way to name/reference specific parts of the compilation pipeline. -Xfoo used > to work OK for the linear 'standar

[PATCH] D125904: [Cuda] Use fallback method to mangle externalized decls if no CUID given

jhuber6 added a comment. In D125904#3532492 , @tra wrote: > This is a moderately serious issue. Some users care about the build > reproducibility. Recompiling the same sources and getting different results > will trigger all sorts of red flags that woul

[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

jhuber6 updated this revision to Diff 431491. jhuber6 added a comment. Go back to old joined method and also change the name to remote `_EQ`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126226/new/ https://reviews.llvm.org/D126226 Files: clang

[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

jhuber6 added inline comments. Comment at: clang/include/clang/Driver/Options.td:826 Group; +def Xoffload_linker : Separate<["-"], "Xoffload-linker">, + HelpText<"Pass to the offload linker">, MetaVarName<"">, tra wrote: > This option still stands out as a s

[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

jhuber6 updated this revision to Diff 431511. jhuber6 added a comment. Merging into a single argument and checking if the joined arg is empty. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126226/new/ https://reviews.llvm.org/D126226 Files: clan

[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

jhuber6 marked 2 inline comments as done. jhuber6 added inline comments. Comment at: clang/include/clang/Driver/Options.td:827 +def Xoffload_linker : JoinedAndSeparate<["-"], "Xoffload-linker">, + HelpText<"Pass to the offload linker identified by ">, + MetaVarName<" ">, Grou

[PATCH] D125904: [Cuda] Use fallback method to mangle externalized decls if no CUID given

jhuber6 added a comment. In D125904#3532608 , @tra wrote: > That said, I would consider compiling the same source with different > preprocessor options to be a legitimate use case that we should support. > Explicitly passing cuid would work as a workaro

[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

This revision was automatically updated to reflect the committed changes. jhuber6 marked an inline comment as done. Closed by commit rGf37101983fc9: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker (authored by jhuber6). Changed prior to commit: https://reviews.llvm.org/D12

[PATCH] D125904: [Cuda] Use fallback method to mangle externalized decls if no CUID given

jhuber6 updated this revision to Diff 431666. jhuber6 added a comment. Removing use of the line number, instead replacing it with an 8 character wide hash of the `-D` options passed to the front-end. This should make it sufficiently unique for users compiling the same file with different options

[PATCH] D125904: [Cuda] Use fallback method to mangle externalized decls if no CUID given

jhuber6 added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6845-6846 +llvm::MD5::MD5Result Result; +for (const auto &Arg : PreprocessorOpts.Macros) + Hash.update(Arg.first); +Hash.final(Result); yaxunl wrote: > Are these optio

[PATCH] D125904: [Cuda] Use fallback method to mangle externalized decls if no CUID given

jhuber6 added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6845-6846 +llvm::MD5::MD5Result Result; +for (const auto &Arg : PreprocessorOpts.Macros) + Hash.update(Arg.first); +Hash.final(Result); yaxunl wrote: > jhuber6 wrote:

[PATCH] D125904: [Cuda] Use fallback method to mangle externalized decls if no CUID given

jhuber6 updated this revision to Diff 431702. jhuber6 added a comment. Adding extra commentto mention the hidden requirement that the driver shuold not define a different `-D` option for the host and device. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D125904: [Cuda] Use fallback method to mangle externalized decls if no CUID given

jhuber6 added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6836 + + // If the CUID is not specified we try to generate a unique postfix. + if (getLangOpts().CUID.empty()) { tra wrote: > > However, [CUID] is not always availible. > > The qu

[PATCH] D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging

jhuber6 created this revision. jhuber6 added a reviewer: jdoerfert. jhuber6 added a project: OpenMP. Herald added subscribers: llvm-commits, openmp-commits, cfe-commits, jfb, guansong, yaxunl. Herald added projects: clang, LLVM. jhuber6 requested review of this revision. Herald added a subscriber:

[PATCH] D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging

jhuber6 added a comment. WIP for adding ident_t support to libomptarget. Still breaks some tests, just wanted to get a start. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87946/new/ https://reviews.llvm.org/D87946 ___

[PATCH] D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging

jhuber6 added a comment. In D87946#2283049 , @jdoerfert wrote: > I guess you can try to use `sed` to update the tests. That's what I did for all the clang tests, they all pass as far as I know and I can use the built clang to correctly build offloaded ap

[PATCH] D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging

jhuber6 updated this revision to Diff 293220. jhuber6 added a comment. Herald added subscribers: aaron.ballman, hiraditya. Fixed failing tests from OpenMPOpt. Formatted files which results in a lot of changes showing. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87946/new/ https://rev

[PATCH] D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging

jhuber6 updated this revision to Diff 293238. jhuber6 added a comment. Added ident_t structs to additional runtime functions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87946/new/ https://reviews.llvm.org/D87946 Files: clang/lib/CodeGen/CGOpe

[PATCH] D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging

jhuber6 added a comment. Yeah, I did clang-format since the last patch complained about some formatting in the linter. Didn't expect it to change quite so much. Also for some reason it won't let me quote your comment. Might've been better to ignore it until the final commit so it doesn't clutte

[PATCH] D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging

jhuber6 added a comment. In D87946#2286406 , @grokos wrote: > In D87946#2286024 , @jhuber6 wrote: > >> Added ident_t structs to additional runtime functions. > > Why are we adding the extra parameter to those additi

[PATCH] D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging

jhuber6 added a comment. In D87946#2286434 , @grokos wrote: > Correct, all `__tgt_target_*` functions not ending in `_mapper` are part of > the old interface and we are keeping them for compatibility with older > versions of clang. These older clang vers

[PATCH] D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging

jhuber6 updated this revision to Diff 293550. jhuber6 added a comment. Updated runtime library to have legacy calls into the new functions with source location information. Because the library interface requires C function naming this required adding a suffix to all the functions clang generates

[PATCH] D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging

jhuber6 added a comment. In D87946#2289959 , @grokos wrote: > Yes, this used to be a point of contention within the community. We discussed > the issue sometime ago and the majority of developers was in favor of this > approach (as opposed to e.g. having

[PATCH] D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging

jhuber6 updated this revision to Diff 293725. jhuber6 added a comment. Fixed not passing mappers to new functions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87946/new/ https://reviews.llvm.org/D87946 Files: clang/lib/CodeGen/CGOpenMPRuntime.

[PATCH] D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging

jhuber6 updated this revision to Diff 293748. jhuber6 added a comment. Changing legacy nowait calls to use the new function interface. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87946/new/ https://reviews.llvm.org/D87946 Files: clang/lib/Code

[PATCH] D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging

jhuber6 updated this revision to Diff 294296. jhuber6 added a comment. Added definition for the ident_t struct from kmp.h along with a method to extract the source location information. Checking the target outcome now prints the file location if ident_t location is available. Repository: rG

[PATCH] D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging

jhuber6 updated this revision to Diff 294352. jhuber6 added a comment. Adding message to build with debugging symbols if source location is not found. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87946/new/ https://reviews.llvm.org/D87946 Files:

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