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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

[PATCH] D153123: Fix diag for read-only target features

jhuber6 added inline comments. Comment at: clang/test/OpenMP/driver-openmp-amdgpu.c:8 +// RUN: --offload-device-only -o - 2>&1 | FileCheck --check-prefix=CHECK %s +// CHECK-NOT: warning: feature flag {{.*}} is ignored since the feature is read only jdoerfert

[PATCH] D151349: [HIP] emit macro `__HIP_NO_IMAGE_SUPPORT`

jhuber6 added a comment. I've started seeing these errors when compiling for OpenMP targeting AMDGPU: $ clang input.c -fopenmp --offload-arch=gfx1030 warning: feature flag '+image-insts' is ignored since the feature is read only [-Winvalid-command-line-argument] Any suggestions for what

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

jhuber6 updated this revision to Diff 531791. jhuber6 added a comment. Updating Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152882/new/ https://reviews.llvm.org/D152882 Files: clang/lib/Driver/ToolChains/Clang.cpp

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

jhuber6 added inline comments. Comment at: clang/test/Driver/linker-wrapper.c:48 +// AMDGPU-LINK-ID: clang{{.*}} -o {{.*}}.img --target=amdgcn-amd-amdhsa -mcpu=gfx90a -O2 -Wl,--no-undefined {{.*}}.o {{.*}}.o + // RUN: clang-offload-packager -o %t.out \ yaxunl

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

jhuber6 updated this revision to Diff 531756. jhuber6 added a comment. Hopefully fixing test on Windows. I think it's fine to let the packager bundle mutliple of these now since it's caught in `clang`. So if the user really wants to force it we should allow them to since the bundler format is

[PATCH] D152965: [OpenMP] Correctly diagnose conflicting target identifierers for AMDGPU

This revision was automatically updated to reflect the committed changes. Closed by commit rGe96bec9cd8e1: [OpenMP] Correctly diagnose conflicting target identifierers for AMDGPU (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D152965: [OpenMP] Correctly diagnose conflicting target identifierers for AMDGPU

jhuber6 updated this revision to Diff 531531. jhuber6 added a comment. Clang format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152965/new/ https://reviews.llvm.org/D152965 Files: clang/lib/Driver/Driver.cpp

[PATCH] D152965: [OpenMP] Correctly diagnose conflicting target identifierers for AMDGPU

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

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

jhuber6 added a comment. In D152882#4422797 , @yaxunl wrote: > In D152882#4422788 , @jhuber6 wrote: > >> In D152882#4421138 , @yaxunl wrote: >> >>> However, bitcode of

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

jhuber6 added a comment. In D152882#4421138 , @yaxunl wrote: > However, bitcode of target ID gfx90a:xnack+ is allowed to link in bitcode of > target ID gfx90a as long as they are from different containers. So there are > two rules about target ID: 1.

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

jhuber6 updated this revision to Diff 531520. jhuber6 added a comment. Herald added subscribers: kerbowa, jvesely. Adjusting test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152882/new/ https://reviews.llvm.org/D152882 Files:

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

jhuber6 added a comment. In D152882#4421138 , @yaxunl wrote: > The design of target ID put constraints on target ID's that can be embedded > into one executable > https://clang.llvm.org/docs/ClangOffloadBundler.html#bundle-entry-id . For > example,

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

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

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

jhuber6 added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2001 -void CodeGenModule::getDefaultFunctionAttributes(StringRef Name, - bool HasOptnone, - bool

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

jhuber6 added a comment. In D139730#4418630 , @arsenm wrote: > In D139730#3991628 , > @JonChesterfield wrote: > >> We currently require that library for libm, which I'm also not thrilled >> about, but at least

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

jhuber6 updated this revision to Diff 530946. jhuber6 added a comment. Format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152391/new/ https://reviews.llvm.org/D152391 Files: clang/include/clang/CodeGen/CodeGenAction.h

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

jhuber6 updated this revision to Diff 530944. jhuber6 added a comment. Removing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152391/new/ https://reviews.llvm.org/D152391 Files: clang/include/clang/CodeGen/CodeGenAction.h

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

jhuber6 marked an inline comment as not done. jhuber6 added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2105-2106 + llvm::AttrBuilder FuncAttrs(F.getContext()); + GetCPUAndFeaturesAttributes(GlobalDecl(), FuncAttrs, +

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

jhuber6 marked an inline comment as not done. jhuber6 added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2052-2106 +void clang::CodeGen::mergeDefaultFunctionDefinitionAttributes( +llvm::Function , const CodeGenOptions CodeGenOpts, +const LangOptions , const

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

jhuber6 updated this revision to Diff 530919. jhuber6 marked 2 inline comments as done. jhuber6 added a comment. Addressing comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152391/new/ https://reviews.llvm.org/D152391 Files:

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

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

[PATCH] D152461: [LTO] Add a config option to make the final output bitcode

jhuber6 updated this revision to Diff 529682. jhuber6 added a comment. Remove leftover debug Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152461/new/ https://reviews.llvm.org/D152461 Files:

[PATCH] D152461: [LTO] Add a config option to make the final output bitcode

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, MaskRay, fhahn, tra, yaxunl, JonChesterfield, tianshilei1992. Herald added subscribers: ormris, StephenFan, steven_wu, hiraditya, inglorion. Herald added a project: All. jhuber6 requested review of this revision. Herald added

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

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

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

jhuber6 updated this revision to Diff 529557. jhuber6 added a comment. Add a better test to show that attributes are replaced and default attributes are added. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152391/new/

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

jhuber6 updated this revision to Diff 529490. jhuber6 added a comment. Updating, in order to do this starting with bitcode I had to expose a helper that performs this operation using the options directly rather than through the `CodeGenModule`. This should keep the existing interfaces intact by

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGdc81d2a4d5b3: [OpenMP] Fix using the target ID when using the new driver (authored by jhuber6). Changed prior to commit: https://reviews.llvm.org/D150998?vs=523924=529476#toc Repository: rG LLVM

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

jhuber6 added a comment. Scratch that, `ASTContext::getFunctionFeatureMap` is used. I wonder if we could forgo that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152391/new/ https://reviews.llvm.org/D152391

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

jhuber6 added a comment. Just realized this is probably going to be a bit more painful. the attribute propagation pass requires a `CodeGenModule` which isn't built without an `ASTContext` so it's not available here. Nothing those functions do explicitly requires the full `CGM`, they only use

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

jhuber6 added a comment. In D152391#4404106 , @tra wrote: >> clang in.bc -Xclang -mlink-builtin-bitcode -Xclang libdevice.10.bc > > If that's something we intend to expose to the user, should we consider > promoting it to a top-level driver option? I'm

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

jhuber6 created this revision. jhuber6 added reviewers: tra, yaxunl, jdoerfert, tianshilei1992, MaskRay, JonChesterfield, phosek. Herald added a project: All. jhuber6 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Clang provides the

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

jhuber6 added a comment. In D150998#4403444 , @yaxunl wrote: > what happens to > > clang -xhip a.c --offload-arch=gfx90a:xnack+ --offload-arch=gfx90a:xnack- > --offload-new-driver -fgpu-rdc > clang -xhip b.c --offload-arch=gfx90a:xnack+

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

jhuber6 added a comment. In D150998#4403401 , @yaxunl wrote: > In D150998#4403359 , @jhuber6 wrote: > >> Can we use this approach for now and land this? It makes the "new driver" >> less broken than it currently

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

jhuber6 added a comment. Can we use this approach for now and land this? It makes the "new driver" less broken than it currently is as it supports target ID compilation in the general term. Fixing the merging rules will be a rather large overhaul so I'd like this to work in the meantime. This

[PATCH] D151839: [LinkerWrapper] Fix static library symbol resolution

This revision was automatically updated to reflect the committed changes. Closed by commit rGa26bd95325f1: [LinkerWrapper] Fix static library symbol resolution (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151839/new/

[PATCH] D151839: [LinkerWrapper] Fix static library symbol resolution

jhuber6 added inline comments. Comment at: clang/test/Driver/linker-wrapper-libs.c:27 // // Check that we extract a static library defining an undefined symbol. // tra wrote: > jhuber6 wrote: > > tra wrote: > > > How does this test test the functionality of

[PATCH] D151839: [LinkerWrapper] Fix static library symbol resolution

jhuber6 updated this revision to Diff 527240. jhuber6 added a comment. Fix test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151839/new/ https://reviews.llvm.org/D151839 Files: clang/test/Driver/linker-wrapper-libs.c

[PATCH] D151839: [LinkerWrapper] Fix static library symbol resolution

jhuber6 added inline comments. Comment at: clang/test/Driver/linker-wrapper-libs.c:27 // // Check that we extract a static library defining an undefined symbol. // tra wrote: > How does this test test the functionality of the undefined symbol? E.g. how >

[PATCH] D151839: [LinkerWrapper] Fix static library symbol resolution

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, tianshilei1992, JonChesterfield, tra, yaxunl. 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

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

jhuber6 added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:8475 "triple=" + TC->getTripleString(), -"arch=" + Arch.str(), +"arch=" + getProcessorFromTargetID(TC->getTriple(), Arch).str(), "kind=" + Kind.str(),

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

jhuber6 added a comment. We should at least be able to `reinterpret_cast` between cases we know are compatible, as the OpenCL check does. One of the problems with the numerical address space is it doesn't have any information to know if that's strictly legal or not. I'm not sure if casting

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

jhuber6 added a comment. In D151087#4361367 , @ebevhan wrote: > I don't think the standard argument really holds. It doesn't mention > restrictions on address spaces because it doesn't have to, since they don't > exist in C++. If they did, I'm pretty

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

jhuber6 added a comment. In D151087#4361237 , @ebevhan wrote: > Clang hasn't needed to formalize all of the address space behavior because > it's managed to piggyback off of the language semantics provided by OpenCL, > and no targets really have had a

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

jhuber6 added a comment. In D151087#4361081 , @arsenm wrote: > They bypass all semantic checks. For example if you declare something as > address space 4, it will let you write to it unlike __constant__. It will let > you place stack objects in globals

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGba9590d8bc76: [Clang][Docs] Add help test to `-march` and `-mcpu` to suggest `-mcpu=help` (authored by jhuber6). Repository: rG LLVM Github

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

[PATCH] D150807: [Clang] Remove direct linking of offloading runtimes from the arch tools

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG66da9ec073ff: [Clang] Remove direct linking of offloading runtimes from the arch tools (authored by jhuber6). Repository: rG LLVM Github Monorepo

[PATCH] D150807: [Clang] Remove direct linking of offloading runtimes from the arch tools

jhuber6 updated this revision to Diff 523125. jhuber6 added a comment. Remove now unused definitions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150807/new/ https://reviews.llvm.org/D150807 Files: clang/tools/amdgpu-arch/AMDGPUArch.cpp

[PATCH] D150807: [Clang] Remove direct linking of offloading runtimes from the arch tools

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

[PATCH] D150461: [OpenMP] Naturally align internal global variables in the OpenMPIRBuilder

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGdd02984519ab: [OpenMP] Naturally align internal global variables in the OpenMPIRBuilder (authored by jhuber6). Repository: rG LLVM Github

[PATCH] D150461: [OpenMP] Naturally align internal global variables in the OpenMPIRBuilder

jhuber6 added a comment. In D150461#4338498 , @gchatelet wrote: > quick question, did you try to build the openmp runtime as well to check if > the tests still pass? I'm an OpenMP developer so I always have it built : ). Yes they still pass.

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

jhuber6 added a comment. Where does the runtime implementation of this live? I'm not very familiar with the HIP / hostcall ecosystem. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150427/new/ https://reviews.llvm.org/D150427

[PATCH] D150461: [OpenMP] Naturally align internal global variables in the OpenMPIRBuilder

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, tianshilei1992, ABataev, JonChesterfield, tstellar, gchatelet. Herald added subscribers: sunshaoce, guansong, hiraditya, yaxunl. Herald added a project: All. jhuber6 requested review of this revision. Herald added subscribers:

[PATCH] D150013: [Clang] Respect `-L` options when compiling directly for AMDGPU

This revision was automatically updated to reflect the committed changes. Closed by commit rG027aeec7da67: [Clang] Respect `-L` options when compiling directly for AMDGPU (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D150013: [Clang] Respect `-L` options when compiling directly for AMDGPU

jhuber6 updated this revision to Diff 521202. jhuber6 added a comment. Updating, @yaxunl does this look good? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150013/new/ https://reviews.llvm.org/D150013 Files:

[PATCH] D150013: [Clang] Respect `-L` options when compiling directly for AMDGPU

jhuber6 added a comment. In D150013#4330452 , @MaskRay wrote: > So it seems that there are configurations that we need -L (cross > compilation?). If we forward -L in some configurations, I think it'd be > better to do this consistently. > > The

[PATCH] D150013: [Clang] Respect `-L` options when compiling directly for AMDGPU

jhuber6 added inline comments. Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:546 addLinkerCompressDebugSectionsOption(getToolChain(), Args, CmdArgs); AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA); + Args.AddAllArgs(CmdArgs, options::OPT_L);

[PATCH] D150156: [OpenMP] Fix incorrect interop type for number of dependencies

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe494ebf9d09b: [OpenMP] Fix incorrect interop type for number of dependencies (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES

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