[PATCH] D158582: [AMDGPU] Respect unresolved symbol option if forwarded to linker

jhuber6 added a comment. Remember to clang format. Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:564-575 + // If the user has manually passed -Wl,--unresolved-symbols=* as a linker + // option, we should not add --no-undefined + bool UnresolvedOpt = false; + for (auto

[PATCH] D158582: [AMDGPU] Respect unresolved symbol option if forwarded to linker

jhuber6 added a comment. In D158582#4610023 , @yaxunl wrote: > The `-Wl` and `-Xlinker` options are intended for the host linker and we > intentionally do not pass them to the device linker. > > If users want to pass options to the device linker, they ne

[PATCH] D156368: [OpenMP] Do not always emit unused extern variables

jhuber6 updated this revision to Diff 545168. jhuber6 added a comment. Add OpenMP runtime test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156368/new/ https://reviews.llvm.org/D156368 Files: clang/lib/CodeGen/CGOpenMPRuntime.cpp clang/lib/Co

[PATCH] D156368: [OpenMP] Do not always emit unused extern variables

jhuber6 added inline comments. Comment at: clang/test/OpenMP/declare_target_codegen.cpp:264 -// CHECK-DAG: !{i32 1, !"aaa", i32 0, i32 {{[0-9]+}}} -// CHECK-DAG: !{i32 1, !"ccc", i32 0, i32 {{[0-9]+}}} // CHECK-DAG: !{{{.+}}virtual_foo tianshilei1992 wrote: > Si

[PATCH] D156368: [OpenMP] Do not always emit unused extern variables

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG141c4e7a9403: [OpenMP] Do not always emit unused extern variables (authored by jhuber6). Herald added a project: OpenMP. Herald added a subscriber: o

[PATCH] D156816: [Clang] Make generic aliases to OpenCL address spaces

jhuber6 created this revision. jhuber6 added reviewers: ebevhan, arsenm, JonChesterfield, jdoerfert, tianshilei1992, tra, yaxunl, rjmccall. Herald added subscribers: jeroen.dobbelaere, Naghasan, ldrumm, arichardson, Anastasia. Herald added a reviewer: aaron.ballman. Herald added a project: All. j

[PATCH] D156816: [Clang] Make generic aliases to OpenCL address spaces

jhuber6 updated this revision to Diff 546149. jhuber6 added a comment. Fix typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156816/new/ https://reviews.llvm.org/D156816 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrD

[PATCH] D156816: [Clang] Make generic aliases to OpenCL address spaces

jhuber6 added a comment. In D156816#4551299 , @arsenm wrote: > I don't really see the point of doing this. These introduce ambiguous > terminology. The reason you need the attributes is basically for FFI to > opencl code, so might as well make the speci

[PATCH] D156816: [Clang] Make generic aliases to OpenCL address spaces

jhuber6 added a comment. In D156816#4551338 , @yaxunl wrote: >> FFI isn't the reason you'd use these, it's for generic access to the actual >> backend. E.g. an `addrspace(3)` global is local memory, if it's external >> it's dynamic. Having these named i

[PATCH] D156816: [Clang] Make generic aliases to OpenCL address spaces

jhuber6 added a comment. In D156816#4551409 , @Anastasia wrote: > Why not to just use target address space and define it to some macro with > desirable spelling? > > I don't think renaming OpenCL address space to something else makes sense. It > might m

[PATCH] D156816: [Clang] Make generic aliases to OpenCL address spaces

jhuber6 added a comment. In D156816#4551500 , @jdoerfert wrote: > Macros seems to be good enough. If we really need clang attributes, we need > new docs, and naming convention etc. Yeah an alternative would be a new set of attributes, then we make the o

[PATCH] D156363: [Driver] -###: exit with code 1 if hasErrorOccurred

jhuber6 added a comment. Also seems to have impacted one of the AMDGPU bots but not the other, not sure why https://lab.llvm.org/staging/#/builders/247/builds/4145. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156363/new/ https://reviews.llvm.org

[PATCH] D156886: [CUDA][HIP] Reorganize options for documentation

jhuber6 accepted this revision. jhuber6 added a comment. This revision is now accepted and ready to land. I think this is good, there's been some confusion around which ones goes to what. Comment at: clang/include/clang/Driver/Options.td:1021 +def offload_host_device : Flag<["

[PATCH] D156363: [Driver] -###: exit with code 1 if hasErrorOccurred

jhuber6 added a comment. Clang :: Driver/amdgpu-hip-system-arch.c Clang :: Driver/cuda-bad-arch.cu Clang :: Driver/hip-autolink.hip Clang :: Driver/hip-binding.hip Clang :: Driver/hip-cuid-hash.hip Clang :: Driver/hip-cuid.hip Clang :: Driver/hip-default-gpu-arch.hip Clang :: Drive

[PATCH] D156363: [Driver] -###: exit with code 1 if hasErrorOccurred

jhuber6 added a comment. Pretty sure most of this is `hip` returning an error if it can't find `ROCm`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156363/new/ https://reviews.llvm.org/D156363 ___ cfe-c

[PATCH] D156363: [Driver] -###: exit with code 1 if hasErrorOccurred

jhuber6 added a comment. In D156363#4554687 , @yaxunl wrote: > In D156363#4554435 , @jhuber6 wrote: > >> Clang :: Driver/amdgpu-hip-system-arch.c >> Clang :: Driver/cuda-bad-arch.cu >> Clang :: Driver/hip-au

[PATCH] D156363: [Driver] -###: exit with code 1 if hasErrorOccurred

jhuber6 added a comment. In D156363#4554790 , @ro wrote: > In D156363#4553043 , @ro wrote: > >> It seems the latest commit of this patch has (re-)introduced two failures on >> the Solaris/amd64 buildbot >>

[PATCH] D156363: [Driver] -###: exit with code 1 if hasErrorOccurred

jhuber6 added a comment. In D156363#4554913 , @yaxunl wrote: > Thanks. I will wait for your patch. You can leave the tricky ones to me if > you would like. e.g. the rocm-detect.hip You can go ahead and fix that one then. Repository: rG LLVM Github M

[PATCH] D156930: [Clang] Fix Offloading related tests after D156363

jhuber6 created this revision. jhuber6 added reviewers: MaskRay, yaxunl, JonChesterfield, tra, jdoerfert, ronlieb, jplehr. Herald added subscribers: mattd, asavonic, ormris, kerbowa, steven_wu, hiraditya, jvesely. Herald added a project: All. jhuber6 requested review of this revision. Herald adde

[PATCH] D156928: [Clang][AMDGPU] Fix handling of -mcode-object-version=none arg

jhuber6 added inline comments. Comment at: clang/include/clang/Basic/TargetOptions.h:90 +COV_Default = 400, +COV_MAX = 500 }; Typically we just put a `COV_LAST` to indicate that it's over the accepted enumerations. Comment at: clang

[PATCH] D156928: [Clang][AMDGPU] Fix handling of -mcode-object-version=none arg

jhuber6 added a comment. In D156928#4555121 , @yaxunl wrote: > `-mcode-object-version=none` was intentionally designed to work with `clang > -cc1` only, since it does not work with clang driver if users link with > device library. Device library can sti

[PATCH] D156930: [Clang] Fix Offloading related tests after D156363

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGbc080221b3a2: [Clang] Fix Offloading related tests after D156363 (authored by jhuber6). Changed prior to commit: https://reviews.llvm.org/D156930?

[PATCH] D156936: [Clang] Increase default architecture from sm_35 to sm_52

jhuber6 created this revision. jhuber6 added reviewers: tra, jdoerfert, yaxunl, jlebar. Herald added a project: All. jhuber6 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We previously defaulted to `sm_35` for the purpose of unspecified a

[PATCH] D156936: [Clang] Increase default architecture from sm_35 to sm_52

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGab202aa7004a: [Clang] Increase default architecture from sm_35 to sm_52 (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D139730: [OpenMP][DeviceRTL][AMDGPU] Support code object version 5

jhuber6 added a comment. Could you explain briefly what the approach here is? I'm confused as to what's actually changed and how we're handling this difference. I thought if this was just the definition of some builtin function we could just rely on the backend to figure it out. Why do we need

[PATCH] D139730: [OpenMP][DeviceRTL][AMDGPU] Support code object version 5

jhuber6 added inline comments. Comment at: openmp/libomptarget/plugins-nextgen/amdgpu/utils/UtilitiesRTL.h:36 -// The implicit arguments of AMDGPU kernels. -struct AMDGPUImplicitArgsTy { - uint64_t OffsetX; - uint64_t OffsetY; - uint64_t OffsetZ; - uint64_t HostcallPtr; -

[PATCH] D139730: [OpenMP][DeviceRTL][AMDGPU] Support code object version 5

jhuber6 added a comment. In D139730#4561573 , @arsenm wrote: > In D139730#4561540 , @jhuber6 wrote: > >> Could you explain briefly what the approach here is? I'm confused as to >> what's actually changed and how

[PATCH] D156928: [Clang][AMDGPU] Fix handling of -mcode-object-version=none arg

jhuber6 added a comment. In D156928#4562239 , @JonChesterfield wrote: > Or, the front end could define those objects directly, without importing IR > files that define the objects with the content clang used to choose the > object file. E.g. instead of

[PATCH] D139730: [OpenMP][DeviceRTL][AMDGPU] Support code object version 5

jhuber6 added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:17138 +///and use its value for COV_4 or COV_5 approach. It is used for +///compiling device libraries in ABI-agnostic way. +/// Comment at: cla

[PATCH] D156014: [Clang][NVPTX] Permit use of the alias attribute for NVPTX targets

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

[PATCH] D156014: [Clang][NVPTX] Permit use of the alias attribute for NVPTX targets

jhuber6 updated this revision to Diff 547871. jhuber6 added a comment. Update to check the SDK version. Permit this if there is not passed in SDK version so that freestanding targets can still target CUDA and assume it's supported. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D156014: [Clang][NVPTX] Permit use of the alias attribute for NVPTX targets

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0ba9aec38faa: [Clang][NVPTX] Permit use of the alias attribute for NVPTX targets (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANG

[PATCH] D156014: [Clang][NVPTX] Permit use of the alias attribute for NVPTX targets

jhuber6 added a comment. In D156014#4567363 , @steven_wu wrote: > This breaks macOS bot: > https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/36900/testReport/junit/Clang/SemaCUDA/alias_cu/ I should've fixed that already. Is it still

[PATCH] D157438: [OpenMP] Ensure wrapper headers are included on both host and device

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

[PATCH] D157438: [OpenMP] Ensure wrapper headers are included on both host and device

jhuber6 added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1190-1191 // the resource directory at clang/lib/Headers/llvm_libc_wrappers. -if (C.getActiveOffloadKinds() == Action::OFK_None) { +if ((getToolChain().getTriple().isNVPTX() || +

[PATCH] D157438: [OpenMP] Ensure wrapper headers are included on both host and device

jhuber6 added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1190-1191 // the resource directory at clang/lib/Headers/llvm_libc_wrappers. -if (C.getActiveOffloadKinds() == Action::OFK_None) { +if ((getToolChain().getTriple().isNVPTX() || +

[PATCH] D157438: [OpenMP] Ensure wrapper headers are included on both host and device

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG61709bbae37a: [OpenMP] Ensure wrapper headers are included on both host and device (authored by jhuber6). Repository: rG LLVM Github Monorepo CHA

[PATCH] D156816: [Clang] Make generic aliases to OpenCL address spaces

jhuber6 added a comment. In D156816#4574190 , @arsenm wrote: > Probably should just wrap uses in macros for now In clang? Or just have users deal with `opencl_` on everything and rename it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D154378: [LinkerWrapper] Set the GPU LTO job to be freestanding

jhuber6 created this revision. jhuber6 added reviewers: JonChesterfield, jdoerfert, yaxunl, tianshilei1992. Herald added a subscriber: inglorion. Herald added a project: All. jhuber6 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The LTO c

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

jhuber6 updated this revision to Diff 537535. jhuber6 added a comment. Fix guard on the headers for offloading languages Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154036/new/ https://reviews.llvm.org/D154036 Files: clang/lib/Driver/ToolChain

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

jhuber6 added inline comments. Comment at: libc/include/CMakeLists.txt:8 +if(LIBC_TARGET_ARCHITECTURE_IS_GPU) + include(GetClangResourceDir) +endif() sivachandra wrote: > Where does this come from? It's a global CMake module that LLVM provides in `cmake/Modules

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

jhuber6 updated this revision to Diff 537787. jhuber6 added a comment. Changing this to only apply to OpenMP for now. It breaks CUDA / HIP builds because they already have forward declarations of things like `malloc` or `memcpy` on the GPU that conflict. We'll need to clean those up later. Repos

[PATCH] D154591: [OpenMP][OMPIRBuilder] Rename IsEmbedded and IsTargetCodegen flags

jhuber6 accepted this revision. jhuber6 added a comment. This revision is now accepted and ready to land. Noisy change but shouldn't really affect much. Names area little overloaded here so it's good to be more specific. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa4a26374aa11: [libc] Add support for creating wrapper headers for offloading in clang (authored by jhuber6). Repository: rG LLVM Github Monorepo

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

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

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

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

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

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

[PATCH] D154591: [OpenMP][OMPIRBuilder] Rename IsEmbedded and IsTargetCodegen flags

jhuber6 added a comment. Yeah that happens sometimes, if it's working on your system it's safe to assume it's fine. Worst case scenario you can always revert, it's not a big deal. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154591/new/ https://r

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

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

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

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

[PATCH] D154850: [libc] Remove GPU string functions incompatible with C++

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, JonChesterfield, sivachandra, lntue, michaelrj, ronlieb. Herald added projects: libc-project, All. Herald added a subscriber: libc-commits. jhuber6 requested review of this revision. Herald added a project: clang. Herald added a su

[PATCH] D154850: [libc] Remove GPU string functions incompatible with C++

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb454e7aa7ceb: [libc] Remove GPU string functions incompatible with C++ (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D155211: [NVPTX] Add initial support for '.alias' in PTX

jhuber6 created this revision. jhuber6 added reviewers: tra, arsenm, jlebar, kushanam. Herald added subscribers: mattd, gchakrabarti, asavonic, jeroen.dobbelaere, hiraditya. Herald added a reviewer: aaron.ballman. Herald added a project: All. jhuber6 requested review of this revision. Herald added

[PATCH] D155211: [NVPTX] Add initial support for '.alias' in PTX

jhuber6 updated this revision to Diff 540062. jhuber6 added a comment. Remove changes in `clang` that I forgot to remove to keep this restricted to the codegen. Afterwards we can remove the sema in clang and test it there. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D139730: [OpenMP][DeviceRTL][AMDGPU] Support code object version 5

jhuber6 added inline comments. Comment at: openmp/libomptarget/plugins-nextgen/amdgpu/utils/UtilitiesRTL.h:49 -static_assert(sizeof(AMDGPUImplicitArgsTy) == 56, - "Unexpected size of implicit arguments"); +enum IMPLICITARGS : uint32_t { + COV4_SIZE = 56, -

[PATCH] D139730: [OpenMP][DeviceRTL][AMDGPU] Support code object version 5

jhuber6 added inline comments. Comment at: openmp/libomptarget/plugins-nextgen/amdgpu/utils/UtilitiesRTL.h:49 -static_assert(sizeof(AMDGPUImplicitArgsTy) == 56, - "Unexpected size of implicit arguments"); +enum IMPLICITARGS : uint32_t { + COV4_SIZE = 56, -

[PATCH] D157738: [OpenMP] Emit offloading entries for indirect target variables

This revision was automatically updated to reflect the committed changes. Closed by commit rG9da61aed751e: [OpenMP] Emit offloading entries for indirect target variables (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157738/new/

[PATCH] D139730: [OpenMP][DeviceRTL][AMDGPU] Support code object version 5

jhuber6 added a comment. Just a few more nits. I think it's looking fine but I haven't tested it. Anyone else? Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:406 + // pass on -mllvm options to the clang + for (const opt::Arg *Arg : Args.filtered(OPT_ml

[PATCH] D139730: [OpenMP][DeviceRTL][AMDGPU] Support code object version 5

jhuber6 accepted this revision. jhuber6 added a comment. This revision is now accepted and ready to land. I think it's fine now given that it's passing tests. Others feel free to comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139730/new/ h

[PATCH] D153924: [OpenMP] Allow exceptions in target regions when offloading to GPUs

jhuber6 added a comment. The libcxx tests are always broken randomly in my experience. I wouldn't worry about it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153924/new/ https://reviews.llvm.org/D153924 _

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

jhuber6 added a comment. This caused some linking errors with the GPU libc test suite, see https://lab.llvm.org/staging/#/builders/247/builds/5659. clang++: error: ld.lld command failed with exit code 1 (use -v to see invocation) [331/473] Linking CXX executable libc/test/src/__support/lib

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

jhuber6 added a subscriber: sivachandra. jhuber6 added a comment. In D112921#4624580 , @wangpc wrote: > It seems that the linker can't find sized deallocation (no support in the > environment or AMDGPU libraries?). We should have some implementations he

[PATCH] D159118: [libc] Implement the 'clock()' function on the GPU

jhuber6 created this revision. jhuber6 added reviewers: arsenm, tra, JonChesterfield, jdoerfert, sivachandra, lntue, michaelrj. Herald added subscribers: libc-commits, tpr. Herald added projects: libc-project, All. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits, w

[PATCH] D159118: [libc] Implement the 'clock()' function on the GPU

jhuber6 updated this revision to Diff 554434. jhuber6 added a comment. Address nits and add static check for size of `clock_t` type. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159118/new/ https://reviews.llvm.org/D159118 Files: clang/lib/Head

[PATCH] D158778: [CUDA] Propagate __float128 support from the host.

jhuber6 added a comment. In D158778#4625892 , @tra wrote: > In D158778#4624408 , @ABataev wrote: > >> Just checks removal should be fine > > Looks like OpenMP handles long double and __float128 differently -- it a

[PATCH] D159118: [libc] Implement the 'clock()' function on the GPU

jhuber6 updated this revision to Diff 554797. jhuber6 added a comment. Move the header portion into a common utility header. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159118/new/ https://reviews.llvm.org/D159118 Files: clang/lib/Headers/llvm

[PATCH] D159118: [libc] Implement the 'clock()' function on the GPU

This revision was automatically updated to reflect the committed changes. Closed by commit rG30307a7bb795: [libc] Implement the 'clock()' function on the GPU (authored by jhuber6). Changed prior to commit: https://reviews.llvm.org/D159118?vs=554797&id=554831#toc Repository: rG LLVM Github Mo

[PATCH] D159256: [NFC][Clang] Remove redundant function definitions

jhuber6 added a comment. In D159256#4630410 , @jmmartinez wrote: > @jhuber6 I was wondering if there is a reason you kept 3 versions of > `mergeDefaultFunctionDefinitionAttributes` in > https://reviews.llvm.org/D152391 ? I believe it's because one was

[PATCH] D159256: [NFC][Clang] Remove redundant function definitions

jhuber6 accepted this revision. jhuber6 added a comment. This revision is now accepted and ready to land. In D159256#4630915 , @jmmartinez wrote: > In D159256#4630876 , @jhuber6 wrote: > >> In D159256#4630410

[PATCH] D138598: [OpenMP] Do not add wrapper headers if using '-nogpuinc'

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

[PATCH] D138598: [OpenMP] Do not add wrapper headers if using '-nogpuinc'

This revision was automatically updated to reflect the committed changes. Closed by commit rG37edd910478d: [OpenMP] Do not add wrapper headers if using '-nogpuinc' (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138598/new/ http

[PATCH] D138862: [Clang] Do not set offload kind in a freestanding build

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

[PATCH] D138862: [Clang] Do not set offload kind in a freestanding build

jhuber6 added a comment. In D138862#3955339 , @tra wrote: >> we should expect it to be handled externally. > > Can you give more details on how it will work? Something/somewhere would need > to generate the glue code to register kernels w/ CUDA runtime.

[PATCH] D138862: [Clang] Do not set offload kind in a freestanding build

jhuber6 added a comment. In D138862#3955461 , @JonChesterfield wrote: > Does this change the annotation on kernels compiled from C or C++ with > -ffreestanding? If so, probably want a test showing the change. I have a > vague idea that they pick up a d

[PATCH] D138862: [Clang] Do not set offload kind in a freestanding build

jhuber6 added a comment. In D138862#3955502 , @tra wrote: > However, we probably need some sort of safeguards warning users if they > compile CUDA *kernels* or code w/ global variable in freestanding mode. The behavior of `-ffreestanding` isn't well do

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

jhuber6 added a comment. In D133539#3955099 , @sandoval wrote: > Overall this approach looks good to me, though I'd suggest some (hopefully > minor) changes that would help avoid breaking the ABI. It looks like > `__tgt_bin_desc` is only used by `__tgt

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

jhuber6 updated this revision to Diff 505166. jhuber6 added a comment. Adding release notes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145941/new/ https://reviews.llvm.org/D145941 Files: clang/docs/ReleaseNotes.rst clang/lib/Driver/ToolChai

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG55f38495e38e: [Clang] Always use --no-undefined when linking AMDGPU images (authored by jhuber6). Changed prior to commit: https://reviews.llvm.or

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

jhuber6 accepted this revision. jhuber6 added a comment. This revision is now accepted and ready to land. Herald added a subscriber: jplehr. Pretty sure this is what we discussed in the meeting. So it should be fine to remove the alignment requirement on the declaration since we don't handle it

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

jhuber6 added inline comments. Comment at: clang/lib/Driver/ToolChains/Flang.cpp:128 + if (IsHostOffloadingAction) { +for (size_t i = 1; i < Inputs.size(); ++i) { + if (Inputs[i].getType() != types::TY_Nothing) agozillon wrote: > awarzynski wrote: > > a

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

jhuber6 added inline comments. Comment at: clang/lib/Driver/ToolChains/Flang.cpp:128 + if (IsHostOffloadingAction) { +for (size_t i = 1; i < Inputs.size(); ++i) { + if (Inputs[i].getType() != types::TY_Nothing) agozillon wrote: > jhuber6 wrote: > > agoz

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

jhuber6 accepted this revision. jhuber6 added a comment. LGTM, it's much simpler for now since Flang doesn't support CUDA, HIP, OpenCL, OpenMP, etc. Comment at: flang/test/Driver/omp-frontend-forwarding.f90:1 +! REQUIRES: amdgpu-registered-target + agozillon w

[PATCH] D146326: [LinkerWrapper] Do not extract globals with no offloading language

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

[PATCH] D146326: [LinkerWrapper] Do not extract globals with no offloading language

jhuber6 updated this revision to Diff 506190. jhuber6 added a comment. Remove whitespace Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146326/new/ https://reviews.llvm.org/D146326 Files: clang/test/Driver/linker-wrapper-libs.c clang/tools/clan

[PATCH] D146552: [Clang][OpenMP] Enable device-mapped constexpr class members to not be optimized out

jhuber6 added a comment. We should have a clang test as well Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146552/new/ https://reviews.llvm.org/D146552 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D146552: [Clang][OpenMP] Enable device-mapped constexpr class members to not be optimized out

jhuber6 accepted this revision. jhuber6 added a comment. LG, thanks. If you include `Fixes: ` in the commit message it'll automatically close it by the way. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146552/new/ https://reviews.llvm.org/D146552

[PATCH] D146750: [NVPTX] Set the atomic inling threshold when targeting NVPTX directly

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

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

jhuber6 added a comment. @tra would it be possible to go to the earlier version that simply duplicated a slight amount of logic to introduce the new and separate attribute `nvptx_kernel`? Overloading CUDA's `device` attribute is problematic because it's used and checked in several different con

[PATCH] D146750: [NVPTX] Set the atomic inling threshold when targeting NVPTX directly

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGaf54d1e85285: [NVPTX] Set the atomic inling threshold when targeting NVPTX directly (authored by jhuber6). Repository: rG LLVM Github Monorepo CH

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

jhuber6 updated this revision to Diff 508170. jhuber6 added a comment. Updating to simply add an entirely new attribute again. The existing `CUDAGlobal` attribute does what we want, but it's also highly coupled with the CUDA language. This made it pretty much impossible to find a way to re-use it

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG29a5c3c8fe30: [NVPTX] Introduce attribute to mark kernels without a language mode (authored by jhuber6). Changed prior to commit: https://reviews.

[PATCH] D146326: [LinkerWrapper] Do not extract globals with no offloading language

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

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

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

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

jhuber6 added a comment. I'm not sure if there's a better way to provide these headers. Like if we let the `libc` project output to the Clang resource directory or some other neatly nested directory. Right now this just picks up `bin/clang/../include/llvm-libc`. Repository: rG LLVM Github Mo

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

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

[PATCH] D146326: [LinkerWrapper] Do not extract globals with no offloading language

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb530e1af62be: [LinkerWrapper] Do not extract globals with no offloading language (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANG

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

jhuber6 marked an inline comment as done. jhuber6 added inline comments. Comment at: clang/lib/Basic/Targets/NVPTX.cpp:171 Builder.defineMacro("__NVPTX__"); - if (Opts.CUDAIsDevice || Opts.OpenMPIsDevice) { + if (Opts.CUDAIsDevice || Opts.OpenMPIsDevice || !HostTarget) {

[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)); tra wrote: > Ensuring the right include

[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)); tra wrote: > jhuber6 wrote: > > tra wrot

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

jhuber6 added a comment. In D146973#4225300 , @tschuett wrote: > Could you hide the amdgpu and nvptx somewhere libc here `clang > -print-resource-dir` in two different directories? One for AMD, one for > NVPTX. So, right now this header is installed f

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