[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

JonChesterfield added a comment. In D71241#1782427 , @ABataev wrote: > In D71241#1782425 , @JonChesterfield > wrote: > > > > Explain that you're replacing the function written by the user on the fly > > > by anoth

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

JonChesterfield added a comment. > https://clang.llvm.org/docs/InternalsManual.html#the-ast-library > > Faithfulness¶ > The AST intends to provide a representation of the program that is faithful > to the original source. That's pretty convincing. Repository: rG LLVM Github Monorepo C

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

JonChesterfield added a comment. In D71241#1782846 , @ABataev wrote: > But I suggest to discuss this with Richard Smith. Is the appeal to authority necessary to resolve this? The last few posts by Hal look clear to me. Especially convincing is: > We're

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

JonChesterfield added a comment. Has anyone actually asked Richard to look at this? He isn't subscribed to the diff and may not be watching openmp-dev. I don't think it's reasonable to stall progress on optimising openmp indefinitely. Richard may find it difficult to find time to resolve this.

[PATCH] D70289: [OpenMP][NFCI] Use the libFrontend ProcBindKind in Clang

JonChesterfield accepted this revision. JonChesterfield added a comment. This revision is now accepted and ready to land. Nice cleanup, thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70289/new/ https://reviews.llvm.org/D70289 _

[PATCH] D71830: [OpenMP] Reusable OpenMP context/traits handling

JonChesterfield added a comment. Big patch but looks like a net decrease in complexity. Please could you clang format the areas phabricator is complaining about? Reading through on a browser looks great. I'll take a closer look in a real editor once Christmas is out of the way. Thanks for posti

[PATCH] D74361: [Clang] Undef attribute for global variables

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

[PATCH] D73979: [HIP] Allow non-incomplete array type for extern shared var

JonChesterfield added a comment. In D73979#1857736 , @yaxunl wrote: > BTW this is requested by HIP users, who have similar code for CUDA and HIP. > They found it surprised that nvcc allows it but hip-clang does not. I think I'm one of the HIP users here

[PATCH] D74361: [Clang] Undef attribute for global variables

JonChesterfield updated this revision to Diff 248807. JonChesterfield marked 2 inline comments as done. JonChesterfield added a comment. - Review comments, add tests for private_extern Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74361/new/ https:

[PATCH] D74361: [Clang] Undef attribute for global variables

JonChesterfield added a comment. > I thought err_loader_uninitialized_extern says that the variable cannot have > external linkage? Embarrassing! This was a badly written error message, now fixed to: `external declaration of variable cannot have the 'loader_uninitialized' attribute` The premis

[PATCH] D75788: [WIP][OpenMP] Reuse CUDA wrappers in `nvptx` target regions.

JonChesterfield added a comment. That's less invasive than I feared. Nicely done. It may worth keeping the openmp header wrapper to do architecture dispatch. Something like: #ifndef __CLANG_OPENMP_MATH_DECLARES_H__ #define __CLANG_OPENMP_MATH_DECLARES_H__ #ifndef _OPENMP #error "This

[PATCH] D75917: Expose llvm fence instruction as clang intrinsic

JonChesterfield added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3713 + switch (ord) { + case 0: // memory_order_relaxed + default: // invalid order Interesting, fence can't be relaxed > ‘fence’ instructions take an ordering arg

[PATCH] D75917: Expose llvm fence instruction as clang intrinsic

JonChesterfield added inline comments. Comment at: clang/include/clang/Basic/Builtins.def:1516 +// Builtin to expose llvm fence instruction +BUILTIN(__builtin_memory_fence, "vUiUi", "t") `BUILTIN(__builtin_memory_fence, "vii", "n")`? The other fence intrinsics

[PATCH] D75917: Expose llvm fence instruction as clang intrinsic

JonChesterfield added a subscriber: jdoerfert. JonChesterfield added a comment. @jdoerfert this is one of the two intrinsics needed to drop the .ll source from the amdgcn deviceRTL. The other is atomic_inc. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75917/new/

[PATCH] D75917: Expose llvm fence instruction as clang intrinsic

JonChesterfield added a comment. In D75917#1916160 , @sameerds wrote: > how this builtin fits in with the overall scheme of language-specific and > target-specific details of an atomic operation. For example, is this meant > only for OpenCL? Does it work

[PATCH] D75917: Expose llvm fence instruction as clang intrinsic

JonChesterfield added a comment. In D75917#1916972 , @sameerds wrote: > Well, there is a problem: The LangRef says that scopes are target-defined. > This change says that scopes are defined by the high-level language and > further assumes that OpenCL sco

[PATCH] D74361: [Clang] Undef attribute for global variables

JonChesterfield added a comment. ping @aaron.ballman - does that look right to you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74361/new/ https://reviews.llvm.org/D74361 ___ cfe-commits mailing list

[PATCH] D74361: [Clang] Undef attribute for global variables

JonChesterfield updated this revision to Diff 250263. JonChesterfield added a comment. - Amend diagnostic as suggested, clang-format new lines in SemaKinds.td Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74361/new/ https://reviews.llvm.org/D74361

[PATCH] D74361: [Clang] Undef attribute for global variables

JonChesterfield added a comment. In D74361#1920329 , @aaron.ballman wrote: > Aside from the diagnostic wording, I think this LG to me. However, I'd > appreciate if @rjmccall would also sign off. Thanks! @rjmccall? Comment at: clang/

[PATCH] D74361: [Clang] Undef attribute for global variables

JonChesterfield updated this revision to Diff 250264. JonChesterfield marked an inline comment as done. JonChesterfield added a comment. - undo reformat of existing def Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74361/new/ https://reviews.llvm.o

[PATCH] D75917: Expose llvm fence instruction as clang intrinsic

JonChesterfield added a comment. In D75917#1925617 , @sameerds wrote: > Is it really? The scope argument of the IR fence is a target-specific string: > http://llvm.org/docs/LangRef.html#syncscope > > The change that I see here is assuming a numerical argu

[PATCH] D74361: [Clang] Undef attribute for global variables

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

[PATCH] D74361: [Clang] Undef attribute for global variables

JonChesterfield updated this revision to Diff 250901. JonChesterfield added a comment. - Update C codegen test to match output of trunk Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74361/new/ https://reviews.llvm.org/D74361 Files: clang/include

[PATCH] D74361: [Clang] Undef attribute for global variables

JonChesterfield added a comment. Thanks guys. Patched the C codegen test before landing - minor change on trunk in the last week, orthogonal to this. Delighted to have one less reason to write assembly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D74361: [Clang] Undef attribute for global variables

This revision was automatically updated to reflect the committed changes. Closed by commit rGc45eaeabb77a: [Clang] Undef attribute for global variables (authored by JonChesterfield). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74361/new/ https://r

[PATCH] D74361: [Clang] Undef attribute for global variables

JonChesterfield added a comment. Tests made assumptions about alignment which are invalid on arm, failed a buildbot. The tests don't actually care about alignment, so fixed by dropping the `, align #` part of the patterns. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D74361: [Clang] Undef attribute for global variables

JonChesterfield added a comment. In D74361#1927863 , @thakis wrote: > This breaks tests on Windows: http://45.33.8.238/win/10664/step_7.txt > > Please take a look, and if it takes some time please revert while you > investigate. Thanks! It seems Windows

[PATCH] D74361: [Clang] Uninitialize attribute on global variables

JonChesterfield created this revision. JonChesterfield added reviewers: kcc, rjmccall, rsmith, glider, vitalybuka, pcc, eugenis, vlad.tsyrklevich, jdoerfert, gregrodgers. Herald added a project: clang. Herald added a subscriber: cfe-commits. [Clang] Uninitialize attribute on global variables Ext

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

JonChesterfield added a comment. Patch looks good with the above nits. I'm not totally sure about the callback vs running a separate IR pass after the finalize() call, but when the callback is this simple it looks fine. I like that this preserves the current semantics. Comme

[PATCH] D71830: [OpenMP][Part 2] Use reusable OpenMP context/traits handling

JonChesterfield added a comment. Procedural note - adding someone as a blocking reviewer to someone else's patch isn't great. What if the new reviewer never gets around to looking at the patch? I've adjusted that to non-blocking, but feel free to leave a comment if I've missed something. Repo

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

JonChesterfield accepted this revision. JonChesterfield added a comment. This revision is now accepted and ready to land. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74372/new/ https://reviews.llvm.org/D74372 __

[PATCH] D74513: [OpenMP][NFCI] Use the libFrontend DefaultKind in Clang

JonChesterfield added a comment. Nice. Thank you Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74513/new/ https://reviews.llvm.org/D74513 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:/

[PATCH] D74571: [OpenMP][CUDA] Add CUDA 10.2 support

JonChesterfield added a comment. Do the in tree tests all pass with the 10.2 toolchain? That's not exactly the same as whether it works but is the closest approximation we have available. Assuming yes, this patch seems uncontroversial. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7457

[PATCH] D74571: [OpenMP][CUDA] Add CUDA 10.2 support

JonChesterfield added a comment. Interesting distinction. Should compiling without warning indicate comprehensive support, or merely that we ran the tests and they passed? I assumed the latter on the basis that we probably don't have comprehensive support for cuda 10.1 either. No preference ei

[PATCH] D74571: [OpenMP][CUDA] Add CUDA 10.2 support

JonChesterfield added a comment. @tra that's great context, thank you very much for writing it up. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74571/new/ https://reviews.llvm.org/D74571 ___ cfe-commits mailing list cfe-commits@lists.llvm.

[PATCH] D74361: [Clang] Uninitialize attribute on global variables

JonChesterfield added a comment. It zero initialises on x86, but perhaps by coincidence rather than guarantee. Fair enough regarding reuse of the existing attribute, I'll create a new one. Thanks for the pointers on additional cases to test for too. I'll return with an improved patch. Reposit

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

JonChesterfield added a comment. Can we default to freestanding on, or just document that freestanding is a good idea, instead of hacking with include behaviour directly? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146973/new/ https://reviews.ll

[PATCH] D142174: [OpenMP] Don't set rpath for system paths

JonChesterfield accepted this revision. JonChesterfield added a comment. This revision is now accepted and ready to land. I'm happy with this but agree that "what might be a system path?" is a tricky heuristic. What we want is to exclude the places that the application will search anyway, but th

[PATCH] D118493: Set rpath on openmp executables

JonChesterfield added a comment. Reporting after another round of discussion. I don't have much support from the llvm openmp working group that we should maintain the current divergence from libc++ and the like so I'm backing down. Let's revert this and regress for all users who don't install g

[PATCH] D118493: Set rpath on openmp executables

JonChesterfield added a comment. Duplicating a comment from the commit thread so it's easier for me to find later. You've applied this to the release branches going back as far as 14. It's a user facing breaking change. As in people who have a working openmp toolchain and update to the point r

[PATCH] D118493: Set rpath on openmp executables

JonChesterfield added a comment. That is great news, phabricator's list of branches has mislead me. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118493/new/ https://reviews.llvm.org/D118493 ___

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

JonChesterfield added a reviewer: AMDGPU. JonChesterfield added a comment. Adding the amdgpu reviewer group. This change might be contentious - we currently treat various broken code as calls to trap on the grounds that those functions might not be executed. By analogy, functions that call undef

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

JonChesterfield accepted this revision. JonChesterfield added a comment. This revision is now accepted and ready to land. Thanks, unblocked :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153578/new/ https://reviews.llvm.org/D153578 _

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

JonChesterfield accepted this revision. JonChesterfield added a comment. This revision is now accepted and ready to land. LGTM. As a meta level comment, we have far too many of these binary munging sorts of tools. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

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

JonChesterfield added a comment. 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 involve a specific type of input file, does clang foo.ll -o a.out pass a mcpu flag alon

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

JonChesterfield added a comment. Sorry Greg, we can't have this back in trunk. It's now value add for the ROCm toolchain, and some llvm releases had this feature, but no longer. The consensus in the OpenMP weekly meeting was to require users to hack around with linker flags or environment varia

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

JonChesterfield added a comment. I like the current behaviour of bypass semantic checking and emit IR with the same number on it as my primary use case for feeding freestanding C++ to clang is as a convenient way to emit specific IR and I'm not that bothered about clang detecting mistakes along

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

JonChesterfield added a comment. I don't mind hugely what mechanism is used but would really like clang++ -fopenmp foo.cpp to build a program that runs. How can we preserve that 'works' feature without setting rpath on the binary? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

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

JonChesterfield added a comment. I don't know how this configuration file works. Running clang from the build directory is useful when developing, but to avoid user facing regression we'd need it to run from the install directory. The use case is someone builds trunk or a release on a HPC machi

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

JonChesterfield added a comment. So what is this configuration file? Joseph found a Gentoo blog post https://blogs.gentoo.org/mgorny/2022/10/07/clang-in-gentoo-now-sets-default-runtimes-via-config-file/ and I don't have a clang.cfg file in my install dir Repository: rG LLVM Github Monorepo

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

JonChesterfield added inline comments. Comment at: clang/include/clang/Driver/Options.td:4218-4223 defm openmp_implicit_rpath: BoolFOption<"openmp-implicit-rpath", LangOpts<"OpenMP">, - DefaultTrue, + DefaultFalse, PosFlag, NegFlag, BothFlags<[NoArgumentUnused]>>;

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

JonChesterfield requested changes to this revision. JonChesterfield added a comment. This revision now requires changes to proceed. Marking this as "no" because as far as I can tell it'll stop anyone running openmp built from source which constitutes a severe regression and I am struggling to fi

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

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

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

JonChesterfield accepted this revision. JonChesterfield added a comment. This revision is now accepted and ready to land. NVM the above, all the other call sites to addLTOOptions have that test in them so it must be fine Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

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

JonChesterfield added a comment. If fedora has a specific set of paths it rejects in DT_RUNPATH - and libomptarget is on one of those by default - and user executables find it there - and we can detect we're building on fedora then we can disable the rpath for fedora installs to system root, whi

[PATCH] D149019: [Clang] Accept and forward `-fconvergent-functions` in the driver

JonChesterfield added a comment. Can't reasonably see the semantic change between all the whitespace reformat, please split those two. E.g. use git-clang-format to only fix formatting in the part you're changing, or commit a clang-format only change first and then rebase this. Repository: r

[PATCH] D149019: [Clang] Accept and forward `-fconvergent-functions` in the driver

JonChesterfield accepted this revision. JonChesterfield added a comment. This revision is now accepted and ready to land. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149019/new/ https://reviews.llvm.org/D149019 __

[PATCH] D149028: [Clang] Always pass `-fconvergent-functions` for GPU targets

JonChesterfield added a comment. I think this is sensible. Passing fno-convergent-functions presumably changes the default? I wonder if we should adopt this and then remove the checks for each of the GPU programming models Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

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

JonChesterfield added a comment. I'm trying to pick up the context for this and D95976 . Superficially it looks like lowering variadic functions in the compiler could be used to simplify quite a lot of this, @jdoerfert there's a comment from some time ago which

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

JonChesterfield added a comment. That's cleaner than I expected, thanks. Might be reasonable to factor out the method as an initial NFC then insert the call to it along with the new test case as the functional change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D138504: clang/HIP: Remove __llvm_amdgcn_* wrapper hacks

JonChesterfield accepted this revision. JonChesterfield added a comment. This revision is now accepted and ready to land. Yep, thanks. Anything under __llvm is fair game to change and there's a find& replace fix for anything that decided to call these directly. CHANGES SINCE LAST ACTION https

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

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

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

JonChesterfield accepted this revision. JonChesterfield added a comment. This revision is now accepted and ready to land. This LG, the complicated stuff is in the previous patch. Thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152442/new/ http

[PATCH] D152829: clang: Add start of header test for __clang_hip_libdevice_declares

JonChesterfield added a comment. That seems like something we should fix. Visibility/dso really should be the same default. I don't know what that fp thing is CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152829/new/ https://reviews.llvm.org/D152829 ___

[PATCH] D152867: OpenMP: Add a new test for constantexpr evaluation of math headers

JonChesterfield accepted this revision. JonChesterfield added a comment. This revision is now accepted and ready to land. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152867/new/ https://reviews.llvm.org/D152867 ___ cfe-commits maili

[PATCH] D138141: [amdgpu] Reimplement LDS lowering

JonChesterfield updated this revision to Diff 480484. JonChesterfield added a comment. Herald added subscribers: cfe-commits, libc-commits, libcxx-commits, openmp-commits, lldb-commits, Sanitizers, hanchung, kadircet, jsetoain, Moerafaat, zero9178, pcwang-thead, anlunx, steakhal, mtrofin, Enna1,

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

JonChesterfield added a comment. I am reluctant to add the dependency edge to rocm device libs to openmp's GPU runtime. We currently require that library for libm, which I'm also not thrilled about, but at least you can currently build and run openmp programs (that don't use libm, like much of

[PATCH] D137875: clang/AMDGPU: Drop volatile from builtin_atomic* pointers

JonChesterfield accepted this revision. JonChesterfield added a comment. This revision is now accepted and ready to land. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137875/new/ https://reviews.llvm.org/D137875 ___ cfe-commits maili

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

JonChesterfield added a comment. If we do magic header including, we should check for the freestanding argument and not include them with that set. I would prefer we not include cuda headers into C++ source that isn't being compiled as cuda, and also not link in misc cuda library files. Anyone

[PATCH] D117362: [OpenMP] Remove hidden visibility for declare target variables

JonChesterfield added a comment. If I'm following correctly, this broke the amdgpu buildbot and it has been moved into staging as a workaround. I haven't debugged what breaks but 'DefaultVisibility' is relatively likely to behave differently on cuda vs hsa systems so that's my first guess. =

[PATCH] D117362: [OpenMP] Remove hidden visibility for declare target variables

JonChesterfield added inline comments. Comment at: clang/lib/AST/Decl.cpp:1080 + if (const VarDecl *VD = dyn_cast(D)) +if (OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(VD)) + return LinkageInfo(LV.getLinkage(), DefaultVisibility, false); Possibl

[PATCH] D117246: [OpenMP] Add support for linking AMDGPU images

JonChesterfield added a comment. A bit nervous about the automatic appending of host library search paths for the device link but I see that's pre-existing in nvptx. I'll check whether -### changes behaviour as expected and approve if it does Comment at: clang/lib/Driver/Tool

[PATCH] D117706: [openmp] Unconditionally set march commandline argument

JonChesterfield created this revision. JonChesterfield added a reviewer: jhuber6. Herald added subscribers: kerbowa, guansong, tpr, yaxunl, nhaehnle, jvesely. JonChesterfield requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: cfe-commits, sstefan1. Hera

[PATCH] D117706: [openmp] Unconditionally set march commandline argument

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa9935b5db706: [openmp] Unconditionally set march commandline argument (authored by jhuber6, committed by JonChesterfield). Repository: rG LLVM Git

[PATCH] D117706: [openmp] Unconditionally set march commandline argument

JonChesterfield added a comment. Landed with you as author and me as reviewer as that seems a more accurate statement than the default. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117706/new/ https://reviews.llvm.org/D117706 ___

[PATCH] D117634: [OpenMP] Expand short verisions of OpenMP offloading triples

JonChesterfield accepted this revision. JonChesterfield added a comment. This revision is now accepted and ready to land. LG, thanks! Comment at: clang/lib/Driver/Driver.cpp:778 + // We want to normalize the shortened versions of triples passed in to + // the

[PATCH] D70549: [OPENMP]Fix PR41826: symbols visibility in device code.

JonChesterfield added subscribers: jhuber6, JonChesterfield. JonChesterfield added a comment. Herald added subscribers: asavonic, sstefan1, yaxunl. @ABataev @jhuber6 and I would like to change this to annotate variables with whatever visibility the user asked for. Can you recall if the concern a

[PATCH] D117806: [OpenMP] Change default visibility to protected for device declarations

JonChesterfield accepted this revision. JonChesterfield added a comment. This revision is now accepted and ready to land. Looks great, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117806/new/ https://reviews.llvm.org/D117806 _

[PATCH] D117806: [OpenMP] Change default visibility to protected for device declarations

JonChesterfield added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5830 + // host, makes the system more robust, and improves performance. + if (IsOpenMPDevice) { +CmdArgs.push_back("-fvisibility"); I think we need some more code here

[PATCH] D117806: [OpenMP] Change default visibility to protected for device declarations

JonChesterfield accepted this revision. JonChesterfield added a comment. LG, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117806/new/ https://reviews.llvm.org/D117806 ___ cfe-commits mailing lis

[PATCH] D115283: [AMDGPU] Set "amdgpu_hostcall" module flag if an AMDGPU function has calls to device lib functions that use hostcalls.

JonChesterfield added a comment. The asm variable used by rocm aomp is zero overhead, needs no compiler support and works exactly as one would wish under inlining or code elimination. The main argument against that approach seems to be it's an abi break, much like this patch was, and that it is

[PATCH] D116541: [OpenMP] Introduce new flag to change offloading driver pipeline

JonChesterfield accepted this revision. JonChesterfield added a comment. This revision is now accepted and ready to land. This looks reasonable to me, though I'd prefer we keep the forward declare. The scalar->vector transform is mechanical and the `if (Args.hasArg(options::OPT_fopenmp_new_drive

[PATCH] D116542: [OpenMP] Add a flag for embedding a file into the module

JonChesterfield requested changes to this revision. JonChesterfield added a comment. This revision now requires changes to proceed. I don't think this works for multiple files. The test only tries one and misses all the parsing handling. Comment at: clang/include/clang/Basic/C

[PATCH] D116542: [OpenMP] Add a flag for embedding a file into the module

JonChesterfield added inline comments. Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:4891 GlobalVariable *Used = collectUsedGlobalVariables(M, UsedGlobals, true); for (auto *GV : UsedGlobals) { if (GV->getName() != "llvm.embedded.module" && Thi

[PATCH] D116542: [OpenMP] Add a flag for embedding a file into the module

JonChesterfield added inline comments. Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:4934 assert(Old->hasOneUse() && "llvm.embedded.module can only be used once in llvm.compiler.used"); GV->takeName(Old); here's an assert that this f

[PATCH] D118399: [OpenMP] Only generate runtime flags with host input

JonChesterfield added a comment. There's an existing flag for compile for device only, that's probably close enough to the right condition to not emit these flags. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118399/new/ https://reviews.llvm.org/

[PATCH] D116542: [OpenMP] Add a flag for embedding a file into the module

JonChesterfield added a comment. This might be OK. If multiple translation units still get fed to llvm-link it'll be broken, which might be what we get on amdgpu at present. Not totally clear to me whether this should be compiler.used or .used, as it's used by something after clang but before t

[PATCH] D118493: [WIP]Set rpath on openmp executables

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

[PATCH] D118493: [WIP]Set rpath on openmp executables

JonChesterfield added a comment. Testing manually looks good, provided there's no command line argument involved. -rpath even composes sanely, so doing this plus passing -Wl,rpath= results in multiple embedded rpaths. At a loss as to why changing Options.td is working so poorly, though it may b

[PATCH] D118495: [OpenMP] Accept shortened triples for -Xopenmp-target=

JonChesterfield added a comment. Do we have this same expansion logic in two places now? If so, probably want to factor it out to a function Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118495/new/ https://reviews.llvm.org/D118495 __

[PATCH] D118493: [WIP]Set rpath on openmp executables

JonChesterfield added a comment. My plan is to feed executables to readelf -d. Commandline problems seem to be solvable by clean builds between changes, slow but feasible. Might upset the incremental buildbot Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D118493: [WIP]Set rpath on openmp executables

JonChesterfield updated this revision to Diff 404156. JonChesterfield added a comment. Herald added a subscriber: dang. - Now with commandline argument Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118493/new/ https://reviews.llvm.org/D118493 File

[PATCH] D118493: [WIP]Set rpath on openmp executables

JonChesterfield added a comment. Slightly stalled on testing - I'd like to emit the object and feed it to readelf, something like: `// RUN: %clang %s -o %t && llvm-readelf --dynamic-table %t | FileCheck %s --check-prefixes=CHECK` which errors with cannot find -lomp. I feel there should be a lin

[PATCH] D118493: [WIP]Set rpath on openmp executables

JonChesterfield updated this revision to Diff 404193. JonChesterfield added a comment. - set rpath after libomptarget, reads better than between the two -l flags Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118493/new/ https://reviews.llvm.org/D11

[PATCH] D118493: [WIP]Set rpath on openmp executables

JonChesterfield updated this revision to Diff 404496. JonChesterfield added a comment. - Test rpath and runpath setting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118493/new/ https://reviews.llvm.org/D118493 Files: clang/include/clang/Driver/

[PATCH] D118493: [WIP]Set rpath on openmp executables

JonChesterfield updated this revision to Diff 404497. JonChesterfield added a comment. - composes with user specified rpath flags Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118493/new/ https://reviews.llvm.org/D118493 Files: clang/include/cla

[PATCH] D118493: Set rpath on openmp executables

JonChesterfield updated this revision to Diff 404498. JonChesterfield added a comment. Herald added a project: OpenMP. Herald added a subscriber: openmp-commits. - Disable implicit rpath for the tests so we can be certain the tests aren't picking up unrelated libs Repository: rG LLVM Github M

[PATCH] D118493: Set rpath on openmp executables

JonChesterfield updated this revision to Diff 404499. JonChesterfield added a comment. - rebase and format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118493/new/ https://reviews.llvm.org/D118493 Files: clang/include/clang/Driver/Options.td

[PATCH] D116542: [OpenMP] Add a flag for embedding a file into the module

JonChesterfield added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1760 + for (StringRef OffloadObject : CGOpts.OffloadObjects) { +auto FilenameAndSection = OffloadObject.split(','); +llvm::ErrorOr> ObjectOrErr = Could we have a type he

[PATCH] D116542: [OpenMP] Add a flag for embedding a file into the module

JonChesterfield added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1771 +SmallString<128> SectionName( +{".llvm.offloading.", std::get<1>(FilenameAndSection)}); +llvm::EmbedBufferInModule(*M, **ObjectOrErr, SectionName); OK, so o

<    1   2   3   4   5   6   7   8   9   >