[PATCH] D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++

jdoerfert resigned from this revision. jdoerfert added a comment. This has enough active reviewers as it is. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61634/new/ https://reviews.llvm.org/D61634 ___

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

jdoerfert added a comment. In D71475#1791825 , @cchen wrote: > [...] I found it really hard to update the tests by reading the diagnostic > message. Can I refactor the test a bit (like separate each openmp portion to > be CK1, CK2...) so that I can modif

[PATCH] D71739: [WIP] Use operand bundles to encode alignment assumptions

jdoerfert created this revision. jdoerfert added reviewers: hfinkel, xbolva00, lebedev.ri, nikic, Tyker, rjmccall, spatel. Herald added subscribers: bollu, hiraditya. Herald added projects: clang, LLVM. NOTE: This is a prototype not a finished patch! NOTE: There is a mailing list discussion on th

[PATCH] D71739: [WIP] Use operand bundles to encode alignment assumptions

jdoerfert updated this revision to Diff 234817. jdoerfert added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71739/new/ https://reviews.llvm.org/D71739 Files: clang/lib/CodeGen/CodeGenFunction.cpp clang/test/CodeGen/alloc-al

[PATCH] D70973: [OPENMP50]Treat context selectors as expressions, not just strings.

jdoerfert added a comment. I haven't forgotten about this! With the other two changes to the declare variant going to happen now,as well, I figured we should look at this again from a high-level. My plan now is: 1. Move all of the declare variant context stuff out of clang into the libFronten

[PATCH] D70973: [OPENMP50]Treat context selectors as expressions, not just strings.

jdoerfert added a comment. In D70973#1793061 , @jdoerfert wrote: > I haven't forgotten about this! With the other two changes to the declare > variant going to happen now,as well, I figured we should look at this again > from a high-level. > > My plan no

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

jdoerfert added a comment. In D71830#1794920 , @JonChesterfield wrote: > Big patch but looks like a net decrease in complexity. Please could you clang > format the areas phabricator is complaining about? I'll go over it. (FWIW, I do clang format my pat

[PATCH] D70258: [OpenMP][IR-Builder] Introduce the finalization stack

This revision was automatically updated to reflect the committed changes. Closed by commit rGf9c3c5da19ab: [OpenMP][IR-Builder] Introduce the finalization stack (authored by jdoerfert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70258/new/ https:

[PATCH] D71884: [OpenMP] Fix formatting of OpenMP error message.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. I would have just stripped the last chars of ` Out.str()` but if this passes all the tests I'm fine with it. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG6c5d1f40ff8d: [OpenMP][NFCI] Use the libFrontend ProcBindKind in Clang (authored by jdoerfert). Herald added a subscriber: hiraditya. Herald added a project: LLVM. Changed prior to commit: https://revie

[PATCH] D70290: [OpenMP] Use the OpenMPIRBuilder for "omp parallel"

jdoerfert updated this revision to Diff 235390. jdoerfert added a comment. Herald added subscribers: llvm-commits, hiraditya. Herald added a project: LLVM. Rebase + add a test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70290/new/ https://re

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jdoerfert added a comment. In D74935#1907100 , @jeroen.dobbelaere wrote: > Just to give an example: > > int foo(int* restrict *pA, int* restrict *pB) { > int tmp=**pB; > **pA=42; > return tmp - **pB; // **pA and **pB can refer to the same ob

[PATCH] D75591: [OpenMP] Add firstprivate as a default data-sharing attribute to clang

jdoerfert added inline comments. Comment at: clang/test/OpenMP/parallel_master_codegen.cpp:143 +// CK31: [[A_VAL:%.+]] = alloca i32 +// CK31: call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* {{.+}}, i32 1, void (i32*,

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jdoerfert added a comment. In D74935#1908607 , @jeroen.dobbelaere wrote: > [...] > So, adding the 'noalias' attribute to %pA and %pB will change the behavior > of foo(). > > hmm... this explanation is maybe getting too long ? > The short conclusion sho

[PATCH] D74144: [OPENMP50]Add basic support for array-shaping operation.

jdoerfert added inline comments. Comment at: clang/lib/Parse/ParseExpr.cpp:2891 ArgExprs); } + } else if (!getLangOpts().ObjC && getLangOpts().OpenMP >= 50 && Out of curiosity, why do we prevent Objective C here? I

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jdoerfert added a comment. In D74935#1909054 , @jeroen.dobbelaere wrote: > In D74935#1908665 , @jdoerfert wrote: > > > I think we conflate two things here: > > > > 1. The modifications to the LangRef which should b

[PATCH] D75209: [OPENMP][NVPTX]Fix PR45003: add support for complex functions.

jdoerfert added a comment. I am unsure we need this with the proper math support. Sema patch for that is going for review today. I'll try this out soon. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75209/new/ https://reviews.llvm.org/D75209 __

[PATCH] D75210: [Attr] Allow ParsedAttr to be constructor for any Attribute

jdoerfert updated this revision to Diff 248845. jdoerfert added a comment. Pass parsed attribute kind explicitly Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75210/new/ https://reviews.llvm.org/D75210 Files: clang/include/clang/Sema/ParsedAttr.

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

jdoerfert created this revision. jdoerfert added reviewers: kiranchandramohan, ABataev, RaviNarayanaswamy, gtbercea, grokos, sdmitriev, JonChesterfield, hfinkel, fghanim, aaron.ballman. Herald added subscribers: guansong, bollu, mgorny. Herald added a project: clang. This is a WIP patch to show o

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

jdoerfert marked an inline comment as done. jdoerfert added a comment. In D75788#1910743 , @JonChesterfield wrote: > That's less invasive than I feared. Nicely done. We need to run some more tests to make sure it works as expected but I hope we can com

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

jdoerfert updated this revision to Diff 248937. jdoerfert added a comment. Add complex test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75788/new/ https://reviews.llvm.org/D75788 Files: clang/lib/Driver/ToolChains/Clang.cpp clang/lib/He

[PATCH] D75209: [OPENMP][NVPTX]Fix PR45003: add support for complex functions.

jdoerfert added a comment. In D75209#1910047 , @ABataev wrote: > In D75209#1909976 , @jdoerfert wrote: > > > I am unsure we need this with the proper math support. Sema patch for that > > is going for review today.

[PATCH] D75779: [OpenMP] `omp begin/end declare variant` - part 2, semantic analysis

jdoerfert added a comment. There will be an update later today or tomorrow, I am making the actual target offloading CUDA interoperability "work" right now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75779/new/ https://reviews.llvm.org/D75779

[PATCH] D75779: [OpenMP] `omp begin/end declare variant` - part 2, semantic analysis

jdoerfert planned changes to this revision. jdoerfert added a comment. I'll try to remove the extra namespace and I'll split the `function(is_definition)` matcher out. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75779/new/ https://reviews.llvm.o

[PATCH] D75827: Add new `attribute push` matcher `function(is_definition)`

jdoerfert created this revision. jdoerfert added reviewers: erichkeane, aaron.ballman. Herald added a subscriber: bollu. Herald added a project: clang. This can also be used as a subject for attributes. TODO: Tests are missing. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D75

[PATCH] D75827: Add new `attribute push` matcher `function(is_definition)`

jdoerfert abandoned this revision. jdoerfert added a comment. Not needed anymore. Anyone should feel free to pick this up. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75827/new/ https://reviews.llvm.org/D75827 _

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jdoerfert added a comment. In D74935#1915311 , @jeroen.dobbelaere wrote: > In D74935#1909909 , @jdoerfert wrote: > > > I would say that once we get modeling for indirect restrict we can adapt > > the lang ref acco

[PATCH] D75591: [OpenMP] Add firstprivate as a default data-sharing attribute to clang

jdoerfert added a subscriber: fghanim. jdoerfert added a comment. In D75591#1916433 , @atmnpatel wrote: > Modified the unit test for CodeGen of default(firstprivate) to more > accurately reflect the IR. > > From the perspective of the AST, the variables t

[PATCH] D75210: [Attr] Allow ParsedAttr to be constructor for any Attribute

jdoerfert planned changes to this revision. jdoerfert added a comment. The reason for this patch is not there anymore. I'm fine with postponing this patch until there is a user again, thoughts? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75210/ne

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

jdoerfert updated this revision to Diff 250035. jdoerfert marked an inline comment as done. jdoerfert added a comment. Adjust to new scheme, tested locally with some math functions, seems to work Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75788/n

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

jdoerfert marked 3 inline comments as done. jdoerfert added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:1621 case BuiltinType::LongDouble: -if (getLangOpts().OpenMP && getLangOpts().OpenMPIsDevice) return AuxTarget->getLongDoubleFormat(); -

[PATCH] D75010: [OpenMP] Adding InaccessibleMemOnly and InaccessibleMemOrArgMemOnly for runtime calls.

jdoerfert added a comment. I always thought there was a test for this, turns out I never committed it. Since I did now (1c9c23d60ea7656174ad3d76293c2a90dd25e24f ) you need to rebase and adjust it, sorry. Com

[PATCH] D75591: [OpenMP] Add firstprivate as a default data-sharing attribute to clang

jdoerfert added a comment. I think this is functionally correct even if we pass the pointer for now as it is properly "privatized" in the callee. I will go over this once more (after some sleep) but I think it is all good. Once @fghanim puts the new code path on phab we need to ensure it works

[Diffusion] rG396b7253944e: [OpenMP][Opt] Combine `struct ident_t*` during deduplication

jdoerfert marked an inline comment as done. jdoerfert added inline comments. BRANCHES master /clang/test/OpenMP/PR44893.c:3 > A test with no CHECK lines also passes if you symlink /bin/true to clang :-) > It also doesn't indicate what is being relevant and when it comes to > maintaining / upd

[PATCH] D74144: [OPENMP50]Add basic support for array-shaping operation.

jdoerfert added a comment. My questions are answered, as you have the clang people in the reviewer list and I was only added by my OpenMP hook, I would them to OK this. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74144/new/ https://revie

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

jdoerfert marked an inline comment as done. jdoerfert added inline comments. Comment at: clang/include/clang/AST/OpenMPClause.h:6682 + /// The outermost level of selector sets. + llvm::SmallVector Sets; + rnk wrote: > This is not a good data structure choice. Y

[PATCH] D76173: [OpenMP][NFC] Minimize memory usage and copying of `OMPTraitInfo`s

jdoerfert created this revision. jdoerfert added a reviewer: rnk. Herald added subscribers: guansong, bollu. Herald added a project: clang. See rational here: https://reviews.llvm.org/D71830#1922656 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D76173 Files: clang/include/cl

[PATCH] D75001: [OpenMP][cmake] ignore warning on unknown CUDA version

jdoerfert added a comment. In D75001#1891284 , @Hahnfeld wrote: > In D75001#1887876 , @jdoerfert wrote: > > > I like this way better. I was hoping we could do it in our cmake only :) > > > > Give others a day or so

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

jdoerfert added a comment. D75779 is the proper implementation of the OpenMP standard. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71179/new/ https://reviews.llvm.org/D71179 __

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

jdoerfert abandoned this revision. jdoerfert added a comment. D75779 is the proper implementation of the OpenMP standard. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71241/new/ https://reviews.llvm.org/D71241

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

jdoerfert marked an inline comment as done. jdoerfert added inline comments. Comment at: clang/include/clang/AST/OpenMPClause.h:6682 + /// The outermost level of selector sets. + llvm::SmallVector Sets; + jdoerfert wrote: > rnk wrote: > > This is not a good dat

[PATCH] D76173: [OpenMP][NFC] Minimize memory usage and copying of `OMPTraitInfo`s

jdoerfert added a comment. In D76173#1922916 , @rnk wrote: > lgtm, thanks! > > I also noticed that Parser.h includes OpenMPClause.h just for this class. > OpenMPClause.h is pretty big. It's for this family of related methods: > > /// Parse a property ki

[PATCH] D75210: [Attr] Allow ParsedAttr to be constructor for any Attribute

jdoerfert abandoned this revision. jdoerfert added a comment. Code is good and usable but no users anymore. If anyone wants to pick this up, feel free! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75210/new/ https://reviews.llvm.org/D75210 ___

[PATCH] D76184: [OpenMP][NFC] Remove the need to include `OpenMPClause.h`

jdoerfert created this revision. jdoerfert added a reviewer: rnk. Herald added subscribers: guansong, bollu. Herald added a project: clang. See rational here: https://reviews.llvm.org/D76173#1922916 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D76184 Files: clang/include/cl

[PATCH] D76184: [OpenMP][NFC] Remove the need to include `OpenMPClause.h`

jdoerfert updated this revision to Diff 250396. jdoerfert added a comment. Finish changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76184/new/ https://reviews.llvm.org/D76184 Files: clang/include/clang/AST/Attr.h clang/include/clang/AST/Op

[PATCH] D76211: OpenMP Metadirective with user defined condition

jdoerfert added a comment. I think you forgot to add the tests to the commit :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76211/new/ https://reviews.llvm.org/D76211 ___ cfe-commits mailing list cfe

[PATCH] D76140: [InlineFunction] update attributes during inlining

jdoerfert added inline comments. Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1172 +return true; +return false; + }; `mayThrow` is not sufficient. As with my earlier example, a potential `exit` is sufficient to break this, thus you need

[PATCH] D75010: [OpenMP] Adding InaccessibleMemOnly and InaccessibleMemOrArgMemOnly for runtime calls.

jdoerfert accepted this revision. jdoerfert added a comment. LGTM if the test all pass. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75010/new/ https://reviews.llvm.org/D75010 ___ cfe-commits mailing

[PATCH] D75591: [OpenMP] Add firstprivate as a default data-sharing attribute to clang

jdoerfert added a comment. I'm satisfied with this, @fghanim wdyt? Comment at: clang/lib/Sema/SemaOpenMP.cpp:5102 Diag(P.second->getExprLoc(), diag::err_omp_no_dsa_for_variable) << P.first << P.second->getSourceRange(); Diag(DSAStack->getDefaultDSALocat

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

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

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

This revision was automatically updated to reflect the committed changes. Closed by commit rG1ca740387b9b: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder. (authored by fghanim, committed by jdoerfert). Changed prior to commit: https://reviews.llvm.org/D72304?vs=238858

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

jdoerfert reopened this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Reverted in 9dcfc7cd64abb301124cafaa95661b76a1fc5032 because of breakages and warnings: http://lab.llvm.org:8011/bui

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

jdoerfert marked 4 inline comments as done. jdoerfert added a comment. In D71830#1854989 , @kiranchandramohan wrote: > Had a quick look today. Will spend some more time tomorrow. > Please see a few comments inline. Thanks! I'll rebase asap to address y

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

jdoerfert marked an inline comment as done. jdoerfert added inline comments. Comment at: clang/include/clang/Serialization/ASTRecordReader.h:272 + + template <> OpenMPTraitInfo *readUserType() { return readOpenMPTraitInfo(); } + kiranchandramohan wrote: > jdoerf

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

jdoerfert marked an inline comment as done. jdoerfert added inline comments. Comment at: clang/include/clang/Serialization/ASTRecordReader.h:272 + + template <> OpenMPTraitInfo *readUserType() { return readOpenMPTraitInfo(); } + kiranchandramohan wrote: > jdoerf

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

This revision was automatically updated to reflect the committed changes. Closed by commit rGe8a436c5ea26: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder. (authored by fady , committed by jdoerfert). Changed prior to commit: https://reviews.l

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

jdoerfert marked an inline comment as done. jdoerfert added inline comments. Comment at: clang/include/clang/Serialization/ASTRecordReader.h:272 + + template <> OpenMPTraitInfo *readUserType() { return readOpenMPTraitInfo(); } + jdoerfert wrote: > kiranchandramo

[PATCH] D74262: [clang-offload-bundler] Enable handling of partially-linked fat objects

jdoerfert added a comment. (I'll try to actually review this later but I left a comment below) Comment at: clang/test/Driver/clang-offload-bundler-missing-size-section.cpp:1-44 +// REQUIRES: x86-registered-target +// RUN: %clangxx -c %s -o %t_fat.o +// RUN: %clangxx %t_fat.o -

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

jdoerfert added a comment. FWIW, this would really help us to create an OpenMP device (=GPU) runtime written almost entirely in OpenMP, C++, and very few clang builtins. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74361/new/ https://reviews.llvm

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

jdoerfert created this revision. jdoerfert added reviewers: rogfer01, ABataev, JonChesterfield, kiranchandramohan, fghanim. Herald added subscribers: cfe-commits, guansong, bollu, hiraditya. Herald added projects: clang, LLVM. In order to fix PR44560 and to prepare for loop transformations we now

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

jdoerfert marked 7 inline comments as done. jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:113 + if (llvm::OpenMPIRBuilder *OMPBuilder = CGM.getOpenMPIRBuilder()) +OMPBuilder->finalize(); } fghanim wrote: > Does this mean

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

jdoerfert marked 2 inline comments as done. jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:113 + if (llvm::OpenMPIRBuilder *OMPBuilder = CGM.getOpenMPIRBuilder()) +OMPBuilder->finalize(); } fghanim wrote: > jdoerfert wrot

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

jdoerfert added a comment. In D74372#1870547 , @vsk wrote: > Can this delete `NeedWorkaroundForOpenMPIRBuilderBug` from > llvm/lib/Transforms/Utils/CodeExtractor.cpp? I didn't know about the workaround but that was the plan. I'll verify and add it to t

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

jdoerfert updated this revision to Diff 244002. jdoerfert marked 6 inline comments as done. jdoerfert added a comment. Removed OMPIRBuilder workaround in extractor and addressed comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74372/new/ http

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

jdoerfert added a comment. In D74372#1870620 , @vsk wrote: > Thanks :). IIRC `check-clang` is enough to exercise the relevant code path, > as we get an assert in CodeExtractor without the workaround. (side note: > please don't take my comments here as bl

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

jdoerfert updated this revision to Diff 244005. jdoerfert added a comment. Improve comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74372/new/ https://reviews.llvm.org/D74372 Files: clang/lib/CodeGen/CodeGenFunction.cpp llvm/include/llvm/

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

jdoerfert added a comment. @kiranchandramohan Do you have more input on this one? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71830/new/ https://reviews.llvm.org/D71830 ___ cfe-commits mailing list c

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

jdoerfert marked 4 inline comments as done. jdoerfert added a comment. @kiranchandramohan @JonChesterfield I will address the comments just made, if you think this is otherwise fine it would be good if you accept the patch (it's been around for weeks so it's not really people didn't have a chan

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

jdoerfert added a comment. I think this fixes the bug and allows loop optimizations later, let me know if there is anything else Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74372/new/ https://reviews.llvm.org/D74372 __

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

jdoerfert marked 14 inline comments as done. jdoerfert added inline comments. Comment at: clang/include/clang/Basic/Attr.td:185 +// An argument of type \p type with name \p name. +class GenericPointerArgument : Argument { + string Type = type; aaron.ballman wrot

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG8a56d64d7620: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late (authored by jdoerfert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74372/ne

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

jdoerfert added a comment. One question (below) otherwise this looks good. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def:358 +__OMP_DEFAULT_KIND(shared, 3) +__OMP_DEFAULT_KIND(unknown, 4) + Why 2, 3, 4? Do we really need to specify the value here?

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

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/D74513/new/ https://reviews.llvm.org/D74513

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

jdoerfert added a comment. In D74372#1873623 , @plotfi wrote: > Could this be reverted for the time being so that bots can go green? There > appear to be an awful lot of commits piling up on top of this one. Should be fixed by now but you should revert

[PATCH] D74262: [clang-offload-bundler] Enable handling of partially-linked fat objects

jdoerfert added inline comments. Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:84 " o - object\n" + " oo - object; output file is a list of unbundled objects\n" " gch - precompile

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

jdoerfert added a reviewer: ye-luo. jdoerfert added a comment. > Do we also want to put this patch to 10.0.0 RC2 Yes, if there is nothing else needed to support 10.2 we should for sure do that. > in order to avoid bug 44587 which requires >9.0.1 to be the build compiler? Do we already require C

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

jdoerfert added a comment. In D74571#1875022 , @kkwli0 wrote: > In D74571#1874950 , @jdoerfert wrote: > > > > in order to avoid bug 44587 which requires >9.0.1 to be the build > > > compiler? > > > > Do we already

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

jdoerfert added a comment. In D74513#1874825 , @atmnpatel wrote: > I am unable to commit this myself, can someone commit for me? I'll do that later. Ping me if I forget please. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

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

jdoerfert added inline comments. Comment at: clang/lib/AST/OpenMPClause.cpp:1723 +llvm::APInt CondVal = +Selector.ScoreOrCondition->EvaluateKnownConstInt(ASTCtx); +if (CondVal.isNullValue()) aaron.ballman wrote: > jdoerfert wrote: > >

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

jdoerfert marked 11 inline comments as done. jdoerfert added inline comments. Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:367 } else { -llvm_unreachable("Unknown SimpleArgument type!"); +OS << "if (SA->get" << getUpperName() << "())\n "; +

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

jdoerfert added a comment. In D74571#1876617 , @kkwli0 wrote: > Thanks for all the comments. It makes sense to keep the warning there before > any ptx65 features are added. The warning should also apply to both the CUDA > and OpenMP compile paths. As

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

jdoerfert added a comment. In D74571#1876952 , @kkwli0 wrote: > It turns out that having the warning message also affects the C_SUPPORTS_FPIC > test in `cmake/modules/HandleLLVMOptions.cmake`. As a result, `cmake` thinks > that `-fPIC` is not supported.

[PATCH] D74562: [OpenMP][OMPIRBuilder] Introducing the `OMPBuilderCBHelpers` helper class

jdoerfert added a comment. This is great, thanks a lot! I only have two comments where I am not sure I understand the code/change. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:3134 // TODO: Replace with a generic helper function for emitting body auto BodyGenCB =

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

jdoerfert added a comment. In D74571#1877000 , @kkwli0 wrote: > > I am not sure I understand. Do we need to modify stuff outside of > > `/openmp`? I was hoping it is our CMake that can be adjusted to make this > > work as described earlier. TBH, he warni

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG577c9b02ab57: [OpenMP][NFCI] Use the libFrontend DefaultKind in Clang (authored by atmnpatel, committed by jdoerfert). Changed prior to commit: https://reviews.llvm.org/D74513?vs=244334&id=244815#toc R

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

This revision was automatically updated to reflect the committed changes. Closed by commit rG7438059a9032: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder. (authored by fghanim, committed by jdoerfert). Changed prior to commit: https://reviews.llvm.org/D72304?vs=244249

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

jdoerfert added a comment. I did run clang-format on the OMPIRBuilder files and replaced tabs with spaces. I think you need to teach your editor to use spaces in the future. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72304/new/ https://reviews.

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

jdoerfert added a comment. In D72304#1877676 , @MaskRay wrote: > `test/CodeGen/opt-record-1.c` triggers an assertion failure. I hope the macro reordering in rGa0236de7a927 did the tr

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

jdoerfert added a comment. In D72304#1877855 , @fhahn wrote: > It looks like this patch (or the other related changes committed recently) > may cause ASan failures running `Transforms/OpenMP/gtid.ll ` and > `Transforms/OpenMP/parallel_deletion.ll` > htt

[PATCH] D74562: [OpenMP][OMPIRBuilder] Introducing the `OMPBuilderCBHelpers` helper class

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM assuming the comment below is addressed. Comment at: clang/lib/CodeGen/CodeGenFunction.h:354 + +FinilizationBlock = CGF.getJumpDestInCurrentScope(&FiniBB);

[PATCH] D72304: [OpenMP]{OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

jdoerfert added a comment. Phab is not involved in the merge process, it just is a normal git push on top of the llmv-project. Do you have commit rights already? If not I can commit this for you in a few days. Comment at: llvm/lib/Frontend/OpenMP/OMPConstants.cpp:45 ///{ -

[PATCH] D72304: [OpenMP]{OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

jdoerfert added a comment. The commit title has a stray `{` instead of `[` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72304/new/ https://reviews.llvm.org/D72304 ___ cfe-commits mailing list cfe-comm

[PATCH] D72901: [OpenMP] [DOCS] Update OMP5.0 feature status table [NFC]

jdoerfert added inline comments. Comment at: clang/docs/OpenMPSupport.rst:194 +--+--+--+---+ -| de

[PATCH] D73128: [OPENMP][NVPTX]Add NVPTX specific definitions for new/delete operators.

jdoerfert added a comment. In D73128#1832033 , @ABataev wrote: > A note: this header file requires to include stdlib.h too to see the > declarations of malloc/free. Shall We include it here to fix it? otherwise > the error is reported that malloc and fre

[PATCH] D73128: [OPENMP][NVPTX]Add NVPTX specific definitions for new/delete operators.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Some minor comments but other than that, LGTM. Nit: Clang format the file Comment at: clang/lib/Headers/openmp_wrappers/new:8 + *===---

[PATCH] D72996: [Sema] Attempt to perform call-size-specific `__attribute__((alloc_align(param_idx)))` validation

jdoerfert added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:4489 +// Alignment calculations can wrap around if it's greater than 2**29. +unsigned MaximumAlignment = 536870912; +if (I > MaximumAlignment) erichkeane wrote: >

[PATCH] D73005: [Codegen] If reasonable, materialize clang's `AssumeAlignedAttr` as llvm's Alignment Attribute on call-site function return value

jdoerfert added inline comments. Comment at: clang/test/CodeGen/assume-aligned-and-alloc-align-attributes.c:12 // CHECK-NEXT:[[MASKCOND:%.*]] = icmp eq i64 [[MASKEDPTR]], 0 // CHECK-NEXT:call void @llvm.assume(i1 [[MASKCOND]]) // CHECK-NEXT:ret i8* [[CALL]] ---

[PATCH] D72996: [Sema] Attempt to perform call-size-specific `__attribute__((alloc_align(param_idx)))` validation

jdoerfert added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:4489 +// Alignment calculations can wrap around if it's greater than 2**29. +unsigned MaximumAlignment = 536870912; +if (I > MaximumAlignment) erichkeane wrote: >

[PATCH] D72901: [OpenMP] [DOCS] Update OMP5.0 feature status table [NFC]

jdoerfert added a comment. @kkwli0 I propose you can merge the parts where discussion has reached a consensus while the other parts are resolved. I'm generally fine with this, we can always improve on it further. Comment at: clang/docs/OpenMPSupport.rst:194 +---

[PATCH] D72996: [Sema] Attempt to perform call-size-specific `__attribute__((alloc_align(param_idx)))` validation

jdoerfert added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:4489 +// Alignment calculations can wrap around if it's greater than 2**29. +unsigned MaximumAlignment = 536870912; +if (I > MaximumAlignment) hfinkel wrote: > jd

<    10   11   12   13   14   15   16   17   18   19   >