[PATCH] D79675: [OpenMP][OMPBuilder] Adding Privatization Requirements to OMPIRBuilder

jdoerfert added a comment. In D79675#2032764 , @fghanim wrote: > This is a small patch as it is. splitting it any further would make it very > very small :D Very small is great, then I can accept it fast, finding enough time to read larger patches is p

[PATCH] D79676: [Clang][OpenMP][OMPBuilder] Moving OMP allocation and cache creation code to OMPBuilderCBHelpers

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:878 + [this, VDInit, OriginalAddr, VD, ThisFirstprivateIsLastprivate, + OrigVD, &Lastprivates, IRef, &OMPBuilder]() { // Emit private VarDecl with copy init

[PATCH] D79677: [clang][OpenMP][OMPIRBuilder] Adding some Privatization clauses to OpenMP `parallel` Directive

jdoerfert added a comment. Generally you copied the existing Clang logic, correct? Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:803 - OrigVD); -else - (void)CGM.getOpenMPRuntime().registerTa

[PATCH] D79257: [OPENMP50]Codegen for uses_allocators clause.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM. FWIW, we should directly add new runtime functions we emit into OMPKinds.def. That saves us the trouble of searching for the ones we missed later. Repository: rG LLVM Github Mo

[PATCH] D79675: [OpenMP][OMPBuilder] Adding Privatization Requirements to OMPIRBuilder

jdoerfert added a comment. I left some comments on the type stuff. The rest looks good. I think if you rebase the type stuff on D79739 (which I can merge) we should only need to expand `initializeTypes` to make this work as expected. WDYT? Co

[PATCH] D79675: [OpenMP][OMPBuilder] Adding Privatization Requirements to OMPIRBuilder

jdoerfert added a comment. In D79675#2037484 , @fghanim wrote: > In D79675#2037314 , @jdoerfert wrote: > > > I left some comments on the type stuff. The rest looks good. > > I think if you rebase the type stuff on

[PATCH] D79676: [Clang][OpenMP][OMPBuilder] Moving OMP allocation and cache creation code to OMPBuilderCBHelpers

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Interesting that no tests changed but I guess we just start the move to the IRBUilder. I think there are two minor comments from before that need to be addressed and the alloca thing, o

[PATCH] D79968: StoreInst should store Align, not MaybeAlign

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM, two minor things below. Comment at: clang/lib/CodeGen/CGExpr.cpp:130 + auto *Store = new llvm::StoreInst(Init, Var.getPointer(), false, +

[PATCH] D78862: [IR] Convert null-pointer-is-valid into an enum attribute

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Herald added subscribers: jurahul, kuter. In D78862#2027254 , @arsenm wrote: > In D78862#2012560 , @jdoer

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

jdoerfert added a comment. Drive by, I haven't read the entire history. In D79744#2040208 , @rjmccall wrote: > I don't understand why `noalias` is even a concern if the whole buffer passed > to the kernel is read-only. `noalias` is primarily about provi

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

jdoerfert added a comment. In D79744#2040380 , @rjmccall wrote: > In D79744#2040348 , @jdoerfert wrote: > > > Drive by, I haven't read the entire history. > > > > In D79744#2040208

[PATCH] D80148: [OPENMP50]Add initial support for 'affinity' clause.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM with minor comments. Comment at: clang/lib/Parse/ParseOpenMP.cpp:3458 + EnterScope(Scope::OpenMPDirectiveScope | Scope::DeclScope), + P

[PATCH] D80200: [AST][RecoveryExpr] Fix an assertion crash on openMP.

jdoerfert added a comment. Thx! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80200/new/ https://reviews.llvm.org/D80200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D78155: [OpenMP] Use __OPENMP_NVPTX__ instead of _OPENMP in wrapper headers

jdoerfert added a comment. Herald added a subscriber: sstefan1. ping :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78155/new/ https://reviews.llvm.org/D78155 ___ cfe-commits mailing list cfe-commits

[PATCH] D79675: [OpenMP][OMPBuilder] Adding Privatization Requirements to OMPIRBuilder

jdoerfert added a comment. Herald added a subscriber: sstefan1. What's the status? Can we split the target specific types stuff if this may take a while, other patches depend on that :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79675/new/ http

[PATCH] D131639: [OpenMP] Remove 'stdbool.h' from OpenMP header wrappers

jdoerfert requested changes to this revision. jdoerfert added a comment. This revision now requires changes to proceed. I think the code as is upstream is fine. The test input is problematic. There is no guarantee, or even any argument, that stdbool is not included by the compiler or any header

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

jdoerfert added a comment. If you want to overwrite them, weak/linkonce will work (no _odr). Private/internal will not be overwritten but existing uses will keep the private/internal version, IIRC. I assume you want the former. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

jdoerfert accepted this revision. jdoerfert added a comment. LG, the new remarks need to be addressed in a follow up. Please test for them and make a TODO that they should be optimized away. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102107/new/

[PATCH] D133155: Update the docs about IRC

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133155/new/ https://reviews.llvm.org/D133155 ___

[PATCH] D132550: Changes to code ownership in clang and clang-tidy

jdoerfert added inline comments. Comment at: clang/CodeOwners.rst:226 +| Alexey Bataev +| a.bataev\@hotmail.com (email), ABataev (Phabricator), cilkplus (GitHub) + CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132550/new/ https://reviews.llvm.org/D132

[PATCH] D133578: [OpenMP] Add generation of SIMD align assumptions to OMPIRBuilder

jdoerfert added a comment. Thanks for this patch! I have two drive by comments that should probably be addressed first. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:629 + llvm::ArrayRef AlignedVars, + llvm::Value *Alignment, Valu

[PATCH] D133802: [OpenMP] Remove simplified device runtime handling

jdoerfert added a comment. Remove the option and let's also remove the device flag too while we are at it. Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:76 CGOpenMPRuntimeGPU::ExecutionMode &ExecMode; bool SavedRuntimeMode = false; Remove =

[PATCH] D133802: [OpenMP] Remove simplified device runtime handling

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:1048 CGBuilderTy &Bld = CGF.Builder; - OMPBuilder.createTargetDeinit(Bld, IsSPMD, requiresFullRuntime()); + OMPBuilder.createTargetDeinit(Bld, IsSPMD, true); } So, follow

[PATCH] D133802: [OpenMP] Remove simplified device runtime handling

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, two comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:1048 CGBuilderTy &Bld = CGF.Builder; - OMPBuilder.createTargetDeinit(Bld, IsSPMD, requiresFul

[PATCH] D128685: [OpenMP][NFC] Remove unused check lines in Clang/OpenMP tests

jdoerfert created this revision. jdoerfert added a reviewer: jhuber6. Herald added subscribers: mattd, asavonic, guansong, bollu, yaxunl. Herald added a project: All. jdoerfert requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang. The ch

[PATCH] D128686: [OpenMP][NFC] Reuse check lines for Clang/OpenMP tests

jdoerfert created this revision. jdoerfert added reviewers: jhuber6, JonChesterfield, ABataev, RaviNarayanaswamy, ggeorgakoudis, tianshilei1992. Herald added subscribers: mattd, asavonic, guansong, bollu, yaxunl. Herald added a project: All. jdoerfert requested review of this revision. Herald adde

[PATCH] D128686: [OpenMP][NFC] Reuse check lines for Clang/OpenMP tests

jdoerfert added a comment. I'm not so sure but phab things this is small, so no complaints: F23600771: 2022-06-27_18-06-1656371325.png Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128686/new/ https://reviews.l

[PATCH] D124624: [OpenMP] Add variant extension that applies to declarations

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG. one nit. Comment at: clang/include/clang/Basic/AttrDocs.td:4312 declarations with the same name. The template parameters for the base functions -are used to instan

[PATCH] D128685: [OpenMP][NFC] Remove unused check lines in Clang/OpenMP tests

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG178674e23a71: [OpenMP][NFC] Remove unused check lines in Clang/OpenMP tests (authored by jdoerfert). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D128686: [OpenMP][NFC] Reuse check lines for Clang/OpenMP tests

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb52d33e6de55: [OpenMP][NFC] Reuse check lines for Clang/OpenMP tests (authored by jdoerfert). Repository: rG LLVM Github Monorepo CHANGES SINCE L

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

jdoerfert added a comment. Herald added a subscriber: mattd. reverse ping. Are there outstanding issues with this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102107/new/ https://reviews.llvm.org/D102107 _

[PATCH] D129149: [OMPIRBuilder] Add support for simdlen clause

jdoerfert added a comment. Generally, this is fine. Some nits. The only reason not to accept this right now is the test. Why manual check lines? Wrt. the function signature. As soon as we have more then SIMD directives to check we refactor things. Keep it simple until you need the functionality.

[PATCH] D129149: [OMPIRBuilder] Add support for simdlen clause

jdoerfert added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:608 + /// + /// \param DL Debug location for instructions added by unrolling. + /// \param LoopThe simd loop. psoni2628 wrote: > jdoerfert wrote: > > No debu

[PATCH] D118409: [OpenMPIRBuilder] Remove ContinuationBB argument from Body callback.

jdoerfert added inline comments. Herald added subscribers: anlunx, bzcheeseman. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:70 +BasicBlock *splitBBWithSuffix(IRBuilderBase &Builder, bool CreateBranch, + llvm::Twine Suffix = ".split");

[PATCH] D129149: [OMPIRBuilder] Add support for simdlen clause

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, let's update the test script first though. See below. Comment at: clang/test/OpenMP/irbuilder_simdlen.cpp:1 +// RUN: %clang_cc1 -no-opaque-pointers -fopenmp-enable-

[PATCH] D129149: [OMPIRBuilder] Add support for simdlen clause

jdoerfert added a comment. In D129149#3633627 , @Meinersbur wrote: > Why not have `simdlen` an argument to `applySimd` (which can be null if not > used)? In this case it happens to be just adding some metadata, but for > others such as `unrollLoopParti

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

jdoerfert added a comment. Also, make sure to remove all deviceRTL files and probably reset the autogenerated tests to upstream (and re-generate) before you merge (or reupload). In D102107#3633678 , @dhruvachak wrote: > I rebased and resolved conflict

[PATCH] D129008: [Clang][OpenMP] Fix the issue that globalization doesn't work with byval struct function argument

jdoerfert added a comment. cleanup/destructor test missing. @ABataev WDYT? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129008/new/ https://reviews.llvm.org/D129008 ___ cfe-commits mailing list cfe-com

[PATCH] D129033: [Clang] Use metadata to make identifying embedded objects easier

jdoerfert added a comment. Generally makes sense, some notes though Comment at: clang/test/Frontend/embed-object.ll:17 + +; CHECK: !llvm.embedded.object = !{![[METADATA_1:[0-9]+]], ![[METADATA_2:[0-9]+]]} +; CHECK: ![[METADATA_1]] = !{ptr @[[OBJECT_1]], !".llvm.offloading"} --

[PATCH] D129033: [Clang] Use metadata to make identifying embedded objects easier

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129033/new/ https://reviews.llvm.org/D129033 ___

[PATCH] D129151: [Metadata] Add 'exclude' metadata to add the exclude flags on globals

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129151/new/ https://reviews.llvm.org/D129151 ___

[PATCH] D128816: [OpenMP] Add loop tripcount argument to kernel launch and remove push function

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, but rename the function. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.h:890 + /// the int64 value for the number of iterations of the associated loop. + llvm::Va

[PATCH] D129301: [clang-offload-bundler][NFC] Library-ize ClangOffloadBundler (1/4)

jdoerfert added a subscriber: jhuber6. jdoerfert added a comment. Isn't the offload bundler on it's "way out" (=replaced and then deleted soon)? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129301/new/ https://reviews.llvm.org/D129301 ___

[PATCH] D75788: [OpenMP] Provide math functions in OpenMP device code via OpenMP variants

jdoerfert added inline comments. Comment at: clang/lib/Headers/CMakeLists.txt:145 set(openmp_wrapper_files openmp_wrappers/math.h chapuni wrote: > It doesn't contain , intentional? We don't wrap that yet. Do you need it? Repository: rG LLVM Github Monor

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

jdoerfert added a comment. In D102107#3639551 , @dhruvachak wrote: > Is there an llvm/utils script to update clang tests that have RUN lines at > the top? An example is clang/test/OpenMP/debug_threadprivate_copyin.c. You can create the run lines with t

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

jdoerfert added a comment. In D102107#3639615 , @dhruvachak wrote: > In D102107#3639556 , @jdoerfert > wrote: > >> In D102107#3639551 , @dhruvachak >> wrote: >> >>> Is

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

jdoerfert added a comment. In D102107#3640198 , @dhruvachak wrote: > Thanks. I followed the above steps and regenerated a couple of the AST tests > but they still fail. Perhaps I am missing some options? > > I currently have a handful of clang test fail

[PATCH] D129008: [Clang][OpenMP] Fix the issue that globalization doesn't work with byval struct function argument

jdoerfert added a comment. In D129008#3640194 , @tianshilei1992 wrote: > `callCStructCopyConstructor` is actually for Objective-C…Cannot use it here. Don't we generate copies of things elsewhere already? Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D122352: [OpenMP] Do not create offloading entries for internal or hidden symbols

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122352/new/ https://reviews.llvm.org/D122352 ___

[PATCH] D122403: [OpenMP] Add a sematnic check for updating hidden or internal values

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122403/new/ https://reviews.llvm.org/D122403 ___

[PATCH] D122443: [OpenMP] Replace device kernel linkage with weak_odr

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122443/new/ https://reviews.llvm.org/D122443 ___

[PATCH] D120129: [NVPTX] Enhance vectorization of ld.param & st.param

jdoerfert reopened this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Our internal CI flagged an assertion in `llvm::NVPTXTargetLowering::getFunctionParamOptimizedAlign(llvm::Function const*, llvm::Type*, llvm::DataLayout const&)` last night. Given the er

[PATCH] D118680: [Intrinsics] Add `nocallback` to the default intrinsic attributes

jdoerfert closed this revision. jdoerfert added a comment. closed by a81fff8afd06fab8818db521cc79bf933c700e24 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118680/new/ https://r

[PATCH] D120129: [NVPTX] Enhance vectorization of ld.param & st.param

jdoerfert added a comment. Godbold seems to not have included this: $ cat test.cpp int main(){return 0;} $ clang++ -fopenmp -fopenmp-targets=nvptx64 -Xopenmp-target -march=sm_80 test.cpp ... static bool llvm::isa_impl_cl::doit(const From *) [To = llvm::ConstantAsMetadata, From = const l

[PATCH] D120129: [NVPTX] Enhance vectorization of ld.param & st.param

jdoerfert added a comment. In D120129#3408116 , @kovdan01 wrote: > In D120129#3408088 , @jdoerfert > wrote: > >> Godbold seems to not have included this: >> >> $ cat test.cpp >> int main(){return 0;} >> $ c

[PATCH] D120129: [NVPTX] Enhance vectorization of ld.param & st.param

jdoerfert added a comment. In D120129#3408174 , @kovdan01 wrote: > In D120129#3408168 , @jdoerfert > wrote: > >> You don't need to run it. If you use this command line it doesn't crash? > > Yes, I run the same co

[PATCH] D120129: [NVPTX] Enhance vectorization of ld.param & st.param

jdoerfert added a comment. Here are the null pointers that cause the assertion: !nvvm.annotations = !{!8, !9, !8, !10, !10, !10, !10, !11, !11, !10} !8 = !{null, !"align", i32 8} !9 = !{null, !"align", i32 8, !"align", i32 65544, !"align", i32 131080} !10 = !{null, !"align", i32 16}

[PATCH] D122515: [OpenMP] Add AMDGPU calling convention to ctor / dtor functions

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122515/new/ https://reviews.llvm.org/D122515 ___

[PATCH] D120129: [NVPTX] Enhance vectorization of ld.param & st.param

jdoerfert requested changes to this revision. jdoerfert added a comment. This revision now requires changes to proceed. Please revert this commit. It breaks any code using the cuda.11.0.2 libdevice.bc file, the source of the `null` `nvvm.annotations` and the annotations with 5 arguments. See bel

[PATCH] D120129: [NVPTX] Enhance vectorization of ld.param & st.param

jdoerfert added a comment. In D120129#3410493 , @kovdan01 wrote: > In D120129#3410479 , @jdoerfert > wrote: > >> Please revert this commit. It breaks any code using the cuda.11.0.2 >> libdevice.bc file, the sour

[PATCH] D120129: [NVPTX] Enhance vectorization of ld.param & st.param

jdoerfert added a comment. In D120129#3410512 , @kovdan01 wrote: > In D120129#3410510 , @jdoerfert > wrote: > >> The two assertions introduced here do not hold for the libdevice.bc above. >> So whenever we link

[PATCH] D122469: OpenMP 5.1 - Support 'seq_cst' clause on 'flush' directive

jdoerfert added a comment. I think this contains two distinct changes. Please split. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122469/new/ https://reviews.llvm.org/D122469 ___ cfe-commits mailing lis

[PATCH] D122444: [Driver][Linux] Remove D.Dir+"/../lib" from default search paths for LLVM_ENABLE_RUNTIMES builds

jdoerfert reopened this revision. jdoerfert added a comment. This revision is now accepted and ready to land. So, this broke all OpenMP (offloading) as libomp and (libomptarget) are no longer found by default. (OpenMP is build as ENABLE_RUNTIME.) How do we fix this? Repository: rG LLVM Githu

[PATCH] D122592: [OpenMP] Fix library path missing when using OpenMP

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. s/include/library (commit message and code) add test. Assuming both are easy to fix, LG. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12

[PATCH] D122592: [OpenMP] Fix library path missing when using OpenMP

jdoerfert added a comment. In D122592#3411812 , @MaskRay wrote: > In D122592#3411795 , @ye-luo wrote: > >>> using the multiarch directory >> >> If we can cross compile libomp and libomptarget to the target system.

[PATCH] D122831: [OpenMP] Make the new offloading driver the default

jdoerfert added a comment. I feel we should port some of the clang driver tests rather than running them only for the old one. The old one is on its way out, so ... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122831/new/ https://reviews.llvm.org

[PATCH] D122781: This patch aims to conform AMDGPUOpenMP driver sanitizer changes w.r.t HIPAMD toolchain.

jdoerfert added a comment. Commit name and message should be revisited. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122781/new/ https://reviews.llvm.org/D122781 ___ cfe-commits mailing list cfe-commits

[PATCH] D118804: Revert "[Clang] Propagate guaranteed alignment for malloc and others"

jdoerfert added a comment. Herald added a subscriber: StephenFan. Herald added a project: All. Are we considering relanding this with smaller alignment for smaller mallocs (and friends)? See: https://github.com/llvm/llvm-project/issues/54747#issuecomment-1088420860 Repository: rG LLVM Github

[PATCH] D122760: [OpenMP] Add OpenMP variant extension to keep the unmangled name

jdoerfert added a comment. There is no documentation of the extension and attribute. See also below. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1459 + if (auto *A = ND->getAttr()) +ND = A->getFunction(); std::string MangledName = getMangledNameImpl(*this, GD, ND);

[PATCH] D122760: [OpenMP] Add OpenMP variant extension to keep the unmangled name

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:3062 +if (auto *A = Global->getAttr()) + VariantGlobalsEmitted.insert(A->getFunction()); } jhuber6 wrote: > jdoerfert wrote: > > This looks like you now disable the diagn

[PATCH] D129211: [Clang][Doc] Update the release note for clang

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129211/new/ https://reviews.llvm.org/D129211 ___

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

jdoerfert added a comment. In D102107#3685694 , @dhruvachak wrote: > @jdoerfert With this patch, additional remarks are being generated. Please > check whether the new OMP121 remarks in the following tests are OK. > > Clang :: OpenMP/remarks_parallel_in

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

jdoerfert accepted this revision. jdoerfert added a comment. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105221/new/ https://reviews.llvm.org/D105221 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D104904: [OpenMP][AMDGCN] Initial math headers support

jdoerfert accepted this revision. jdoerfert added a comment. LG from my side Comment at: clang/test/Headers/Inputs/include/cstdlib:29 float fabs(float __x) { return __builtin_fabs(__x); } +#endif JonChesterfield wrote: > jdoerfert wrote: > > estewart08 wrote

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

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102107/new/ https://reviews.llvm.org/D102107

[PATCH] D106301: [OpenMP] Disable trap before unreachable for OpenMP device jobs

jdoerfert created this revision. jdoerfert added reviewers: jhuber6, tianshilei1992, JonChesterfield. Herald added subscribers: guansong, bollu, yaxunl. jdoerfert requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang. We want to fold more

[PATCH] D106301: [OpenMP] Disable trap before unreachable for OpenMP device jobs

jdoerfert updated this revision to Diff 359882. jdoerfert added a comment. Fix copy&paste error Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106301/new/ https://reviews.llvm.org/D106301 Files: clang/lib/Driver/ToolChains/Clang.cpp clang/test/

[PATCH] D106301: [OpenMP] Disable trap before unreachable for OpenMP device jobs

jdoerfert added a comment. In D106301#2888162 , @JonChesterfield wrote: > What's the problem with emitting llvm.trap in various unreachable places? llvm.trap is preserved, thus branches to an llvm.trap are preserved. > Wondering if it also affects tran

[PATCH] D106301: [OpenMP] Disable trap before unreachable for OpenMP device jobs

jdoerfert added a comment. As an example, often end up with code like this right now: %26 = load i32, i32* addrspacecast (i32 addrspace(3)* @execution_param to i32*), align 4, !dbg !39, !tbaa !27 %and.i13.i.i = and i32 %26, 4, !dbg !39 %cmp.i14.not.i.i = icmp eq i32 %and.i13.i.i, 0,

[PATCH] D106301: [OpenMP] Disable trap before unreachable for OpenMP device jobs

jdoerfert added a comment. In D106301#2888203 , @JonChesterfield wrote: > In D106301#2888170 , @jdoerfert > wrote: > >> llvm.trap is preserved, thus branches to an llvm.trap are preserved. > > That's interesting

[PATCH] D106298: [OpenMP] Creating the `NumTeams` and `ThreadLimit` attributes to outlined functions

jdoerfert added a comment. Tests? Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:6559 + if (DefaultValTeams > 0) { +OutlinedFn->addFnAttr(llvm::StringRef("NumTeams"), + std::to_string(DefaultValTeams)); Comme

[PATCH] D106301: [OpenMP] Disable trap before unreachable for OpenMP device jobs

jdoerfert abandoned this revision. jdoerfert added a comment. Replaced by D106308 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106301/new/ https://reviews.llvm.org/D106301 ___

[PATCH] D106298: [OpenMP] Creating the `NumTeams` and `ThreadLimit` attributes to outlined functions

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:6659 } return nullptr; } josemonsalve2 wrote: > Should I default here to 1? Since this is an `omp target` Yes, also above. This will make `omp target` and `omp

[PATCH] D104742: [UpdateCCTestChecks] Implement --global-value-regex

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

[PATCH] D104743: [UpdateCCTestChecks] Implement --global-hex-value-regex

jdoerfert added a comment. I don't understand what we do before, and how this work, is `[[#` special in lit? Also, why i32/64 only, that seems arbitrary, no? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104743/new/ https://reviews.llvm.org/D104743

[PATCH] D104743: [UpdateCCTestChecks] Implement --global-hex-value-regex

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, sure why not. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104743/new/ https://reviews.llvm.org/D104743 ___ cfe-commits mailing

[PATCH] D105191: [Clang][OpenMP] Add support for Static Device Libraries

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

[PATCH] D99350: [OPENMP]Fix PR49649: The introduction of $ref globals is not always valid.

jdoerfert added a comment. I don't understand why the cast makes ptxas happy, can you include a test in the runtime so we can make sure that stays that way and is not some ptxas artifact. After all, we still point to a shared memory symbol from global memory which doesn't make too much sense.

[PATCH] D106496: [OpenMP] Change `__kmpc_free_shared` to include the paired allocation size

jdoerfert added inline comments. Comment at: llvm/test/Transforms/OpenMP/remove_globalization.ll:41 ; CHECK-NEXT:[[TMP0:%.*]] = alloca i8, i64 4, align 1 +; CHECK-NEXT:call void @__kmpc_free_shared(i8* nocapture [[TMP0]], i64 noundef 4) #[[ATTR0]] ; CHECK-NEXT:ret

[PATCH] D106496: [OpenMP] Change `__kmpc_free_shared` to include the paired allocation size

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106496/new/ https://reviews.llvm.org/D106496

[PATCH] D106509: [OpenMP][OpenACC] Implement `hold` map type modifier extension in Clang (1/2)

jdoerfert added a comment. I would propose we prefix these new clauses and such with `ompx_`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106509/new/ https://reviews.llvm.org/D106509 ___ cfe-commits ma

[PATCH] D106542: [OPENMP]Fix PR49787: Codegen for calling __tgt_target_teams_nowait_mapper has too few arguments.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, thx Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106542/new/ https://reviews.llvm.org/D106542 __

[PATCH] D106509: [OpenMP][OpenACC] Implement `hold` map type modifier extension in Clang (1/2)

jdoerfert added a comment. > That's fine for me, but don't the routines use llvm_omp_? That was before we "standardized" `ompx_` for OpenMP 5.2. > Should we also have that prefix in various enumerators in the implementation? > For example, what does OMP_MAP_HOLD become? I'd suggest `OMPX_MAP_H

[PATCH] D105876: OMPIRBuilder for Interop directive

jdoerfert added a comment. you need to squash your commits locally and update this revision with a single commit/diff. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:6323-6327 + if (S.hasClausesOfKind() && + !(S.getSingleClause() || +S.getSingleClause() || +

[PATCH] D106793: [OpenMP] Add a driver flag to enable the new device runtime library

jdoerfert added a comment. We need a driver test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106793/new/ https://reviews.llvm.org/D106793 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:

[PATCH] D106793: [OpenMP] Add a driver flag to enable the new device runtime library

jdoerfert added a comment. > I think changing the BC name is something we can do any time, so just calling > it libomptarget-new is fine for now. Yes, let's not disrupt old workflows. The "default" runtime keeps it's name always and we swap it out at some point and remove the option (or make it

[PATCH] D106799: [OpenMP] Always inline the OpenMP outlined function

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. One test needs to be fixed. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106799/new/ https://reviews.llvm.org/D106799 ___

[PATCH] D106298: [OpenMP] Creating the `omp_target_num_teams` and `omp_target_thread_limit` attributes to outlined functions

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. The codegen changes look sensible and the tests show only attributes changed. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106298/ne

[PATCH] D106793: [OpenMP] Add a driver flag to enable the new device runtime library

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. In D106793#2904771 , @JonChesterfield wrote: > Can we call this something other than new? We don't tend to remove command > line arguments and

[PATCH] D105191: [Clang][OpenMP] Add support for Static Device Libraries

jdoerfert added a comment. `x64 debian > Clang.Driver::fat_archive.cpp Failed` @ABataev @grokos any comments? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105191/new/ https://reviews.llvm.org/D105191

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