[PATCH] D82470: [OpenMP][IRBuilder] Support nested parallel regions

jdoerfert marked 3 inline comments as done. jdoerfert added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:282 + IRBuilder<> AllocaBuilder; + /// Map to remember source location strings fghanim wrote: > What's the benefit of thi

[PATCH] D82470: [OpenMP][IRBuilder] Support nested parallel regions

jdoerfert marked 3 inline comments as done. jdoerfert added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:282 + IRBuilder<> AllocaBuilder; + /// Map to remember source location strings fghanim wrote: > jdoerfert wrote: > > fgha

[PATCH] D82518: [openmp] Use Directive_enumSize instead of OMPD_unknown position

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM, maybe a minor adjustment necessary. Comment at: clang/lib/Basic/OpenMPKinds.cpp:583 OpenMPDirectiveKind DKind) { - assert(DKind <= OMPD_unknown); + assert(

[PATCH] D81678: Introduce frozen attribute at call sites for stricter poison analysis

jdoerfert added a comment. > We don't have any optimizations that infer either "speculatable" or "frozen", > though, so I'm not sure there's any practical impact here. `speculatable` is on the shortlist for the Attributor. Will happen eventually. I am sure `frozen` will follow once it is in. We

[PATCH] D82470: [OpenMP][IRBuilder] Support nested parallel regions

jdoerfert added a comment. In D82470#2116581 , @SouraVX wrote: > Overall this seems great Thanks! I have one minor concern: > This patch seems to do 2 things: Support for nested parallel regions(which > was crashing earlier) and some infrastructure chang

[PATCH] D82722: [OpenMP][IRBuilder] Support nested parallel regions

jdoerfert created this revision. jdoerfert added reviewers: anchu-rajendran, kiranktp, fghanim. Herald added a subscriber: rogfer01. Herald added projects: clang, LLVM. jdoerfert added a child revision: D82470: [OpenMP][IRBuilder] Support allocas in nested parallel regions. During code generation

[PATCH] D82470: [OpenMP][IRBuilder] Support allocas in nested parallel regions

jdoerfert updated this revision to Diff 273961. jdoerfert edited the summary of this revision. jdoerfert added a comment. restrict this to the alloca support Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82470/new/ https://reviews.llvm.org/D82470

[PATCH] D82722: [OpenMP][IRBuilder] Support nested parallel regions

jdoerfert added a comment. In D82722#2118958 , @fghanim wrote: > Thanks for working on this. LGTM. > Did you make any changes other than splitting from D82470 > ? No. Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2082 + const Type *RetTyPtr = RetTy.getTypePtr(); + if (!RetTy->isVoidType() && !RetTyPtr->isRecordType() && + RetAI.getKind() != ABIArgInfo::Indirect) { guiand wrote: > guiand wrote

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

jdoerfert added inline comments. Comment at: llvm/lib/Transforms/Utils/CodeExtractor.cpp:932 case Attribute::NoCfCheck: + case Attribute::NoUndef: break; nikic wrote: > Not familiar with this code, but based on the placement of other similar

[PATCH] D82822: [OpenMP][FIX] Consistently use OpenMPIRBuilder if requested

jdoerfert created this revision. jdoerfert added reviewers: anchu-rajendran, kiranktp, fghanim. Herald added subscribers: sstefan1, guansong, bollu, yaxunl. Herald added a project: clang. When we use the OpenMPIRBuilder for the parallel region we need to also use it to get the thread ID (among oth

[PATCH] D82470: [OpenMP][IRBuilder] Support allocas in nested parallel regions

jdoerfert marked an inline comment as done. jdoerfert added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:282 + IRBuilder<> AllocaBuilder; + /// Map to remember source location strings fghanim wrote: > jdoerfert wrote: > > fgha

[PATCH] D78190: Add Bfloat IR type

jdoerfert added inline comments. Comment at: llvm/include/llvm/IR/Constants.h:790 + static Constant *getFP(Type *ElementType, ArrayRef Elts); /// Return a ConstantVector with the specified constant in each element. Drive by: The documentation of these funct

[PATCH] D78478: [UpdateTestChecks] Add UTC_ARGS support for update_{llc,cc}_test_checks.py

jdoerfert added a comment. I feel there is a lot of good stuff here but it seems to mix two things. A rewrite of the script infrastructure and the UTC_ARGS stuff. If so, do you think we could split them? I feel there are also minor NFC changes that could go in on their own without review, I mar

[PATCH] D78232: [OPENMP50]Codegen for scan directive in simd loops.

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.h:1351 +Exclusive, +Unknown, + }; I think it is not `Unknown` but `None` (or similar) as we know that it is not a scan reduction. Comment at: clang/lib/Cod

[PATCH] D78637: [OPENMP]Fix PR45383: type dependent array subscripts are diagnosed erroneously.

jdoerfert added a comment. Can we merge the test changes into D78643 and continue with D78643 as a fix? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78637/new/ https://reviews.ll

[PATCH] D78643: [OpenMP] Fix false error report of array subscription for templated indexes.

jdoerfert added a comment. Can we create a test case that shows even if it is a dependent type we will eventuall issue an error if it is not an addressable lvalue or array item? If this is the part that needs refactoring to work, we should add the test with a TODO. Repository: rG LLVM Github

[PATCH] D78577: [OPENMP50]Basic support for uses_allocators clause.

jdoerfert added a subscriber: rnk. jdoerfert added inline comments. Comment at: clang/include/clang/Sema/Sema.h:33 #include "clang/AST/NSAPI.h" +#include "clang/AST/OpenMPClause.h" #include "clang/AST/PrettyPrinter.h" I think @rnk might not be happy with this i

[PATCH] D78643: [OpenMP] Fix false error report of array subscription for templated indexes.

jdoerfert added a comment. This looks reasonable to me. @ABataev WDYT? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78643/new/ https://reviews.llvm.org/D78643 ___ cfe-commits mailing list cfe-commits@

[PATCH] D78733: [OPENMP]Use new interface for task reduction.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM. One nit below. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.h:170 ArrayRef ReductionOps); - /// Emits lvalue for a reduction item. + /// Em

[PATCH] D78643: [OpenMP] Fix false error report of array subscription for templated indexes.

jdoerfert added a comment. In D78643#2012212 , @h-joo wrote: > Sorry, I feel like I'm holding back the fix for this bug since I'm stuck on > how to proceed with the build failure, @ABataev would you like to proceed > with the other revision

[PATCH] D78738: [OPENMP50]Support 'task' modifier in reduction clauses.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. I'm assuming we do not have to check that no one uses `task, inscan: + : m`, right? If so, LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

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

jdoerfert added a comment. I think this is an improvement over the status quo and it looks fine to me. @arsenm I agree that we should tie this to the data layout (or at least should try) but I guess there are open questions to answer and code to write. I propose to accept this and work on the DL

[PATCH] D78969: [clang][OpenMP] Fix getNDSWDS for aarch64.

jdoerfert added a comment. Is the NDS and WDS number never visible in the IR, e.g., as part of the name? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78969/new/ https://reviews.llvm.org/D78969 ___ cfe

[PATCH] D78965: [clang][OpenMP] Fix mangling of linear parameters.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Code LGTM. I trust you know about mangling. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:11402 + .getTypeSizeInChars(P->getPoin

[PATCH] D78941: [OPENMP]Allow cancellation constructs in target parallel regions.

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

[PATCH] D78577: [OPENMP50]Basic support for uses_allocators clause.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. A few nits and one question/concern below. Once/If that one is addressed this LGTM. Comment at:

[PATCH] D79034: [OPENMP50]Codegen for reduction clauses with 'task' modifier.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Some nits noted below. Can be done prior to commit. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:2374 +break; + } case OMPRTL__kmpc_alloc: { --

[PATCH] D79186: [OPENMP]Consider 'omp_null_allocator' as a predefined allocator.

jdoerfert added a comment. Herald added a reviewer: aaron.ballman. What would you think about moving the allocator definitions to OMPKinds.def instead of listing them explicitly? As far as I can tell we really only distinguish between predefined and user allocators, right? Repository: rG LLV

[PATCH] D78969: [clang][OpenMP] Fix getNDSWDS for aarch64.

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

[PATCH] D79186: [OPENMP]Consider 'omp_null_allocator' as a predefined allocator.

jdoerfert added a comment. In D79186#2013689 , @ABataev wrote: > In D79186#2013603 , @jdoerfert wrote: > > > What would you think about moving the allocator definitions to OMPKinds.def > > instead of listing them e

[PATCH] D82822: [OpenMP][FIX] Consistently use OpenMPIRBuilder if requested

jdoerfert added a comment. @davezarzycki The bots reported this as well, didn't build MLIR locally, this should have been fixed by 4d83aa4771d84940626d86c883193af390812281 Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D82822: [OpenMP][FIX] Consistently use OpenMPIRBuilder if requested

jdoerfert added a comment. In D82822#2189366 , @hoyFB wrote: > In D82822#2187576 , @jdoerfert wrote: > >> @davezarzycki The bots reported this as well, didn't build MLIR locally, >> this should have been fixed by 4

[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

jdoerfert resigned from this revision. jdoerfert edited reviewers, added: rsmith, rjmccall; removed: jdoerfert, sscalpone. jdoerfert added subscribers: rjmccall, rsmith. jdoerfert added a comment. Herald added a reviewer: jdoerfert. Herald added a reviewer: jdoerfert. In D82317#2185134

[PATCH] D84844: [OpenMP] Ensure testing for versions 4.5 and default - Part 1

jdoerfert added a comment. I think this is good. Let's see if there are objections Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84844/new/ https://reviews.llvm.org/D84844 ___ cfe-commits mailing list cf

[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

jdoerfert added a comment. In D82317#2200789 , @rjmccall wrote: > Are you seriously adding an attribute to literally every argument and return > value? Why is this the right representation? As far as I understand, this should not be literally every argu

[PATCH] D85551: [OpenMP] Split OpenMP/target_map_codegen test [NFC]

jdoerfert added a comment. Wow, cool. I imagine it was hard to split this given the manual check lines. We really need to start using the upgrade scripts here. I'm fine with this, @ABataev WDYT? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85551/

[PATCH] D85551: [OpenMP] Split OpenMP/target_map_codegen test [NFC]

jdoerfert added a comment. In D85551#2203758 , @tra wrote: > In D85551#2203711 , @jdoerfert wrote: > >> Wow, cool. I imagine it was hard to split this given the manual check lines. >> We really need to start using

[PATCH] D85551: [OpenMP] Split OpenMP/target_map_codegen test [NFC]

jdoerfert added inline comments. Comment at: clang/test/OpenMP/target_map_codegen_18.cpp:5 + +///==/// +// RUN: %clang_cc1 -DUSE -DCK19 -verify -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -tri

[PATCH] D85563: [OpenMP 5.0] Fix PR-45212 - Shouldn't error out while using overloaded operator for map clause

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM. Thanks for the quick turnaround time! Nit: We usually avoid the `-` between PR and the number (your commit message). I guess that makes it easier to copy it into `llvm.org/PRX`

[PATCH] D85345: [BuildLibCalls] Add noundef to standard I/O functions

jdoerfert added a comment. In D85345#2205691 , @aqjune wrote: > If things go well, I'll add noundef to other library functions such as utime, > setenv, unsetenv, as well. > For malloc/calloc/realloc/free, I'll think more about them. I guess it is >

[PATCH] D85345: [BuildLibCalls] Add noundef to standard I/O functions

jdoerfert added a comment. In D85345#2205712 , @aqjune wrote: > What about undef or poison is given to malloc? If it should raise UB, the > size argument and returned pointer should be noundef. It is unclear to me if we want to forbid undef to be passed

[PATCH] D80735: [OpenMP][NFC] Reuse OMPIRBuilder `struct ident_t` handling in Clang

jdoerfert marked 2 inline comments as done. jdoerfert added inline comments. Comment at: clang/test/OpenMP/master_taskloop_reduction_codegen.cpp:164 // CHECK:store i32 [[SUB12]], i32* [[DOTCAPTURE_EXPR_9]], -// CHECK:[[TMP65:%.*]] = call i8* @__kmpc_omp_task_alloc(%struc

[PATCH] D80735: [OpenMP][NFC] Reuse OMPIRBuilder `struct ident_t` handling in Clang

jdoerfert added inline comments. Comment at: clang/test/OpenMP/master_taskloop_reduction_codegen.cpp:164 // CHECK:store i32 [[SUB12]], i32* [[DOTCAPTURE_EXPR_9]], -// CHECK:[[TMP65:%.*]] = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* %{{.+}}, i32 [[TMP0]], i32 1, i6

[PATCH] D85099: [UpdateTestChecks] Match unnamed values like "@[0-9]+" and "![0-9]+"

jdoerfert updated this revision to Diff 284537. jdoerfert added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Update clang tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85099/new/ https://reviews.llvm.org/D

[PATCH] D85099: [UpdateTestChecks] Match unnamed values like "@[0-9]+" and "![0-9]+"

jdoerfert updated this revision to Diff 284543. jdoerfert marked an inline comment as done. jdoerfert added a comment. added @MaskRays's tweaks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85099/new/ https://reviews.llvm.org/D85099 Files: clang

[PATCH] D85099: [UpdateTestChecks] Match unnamed values like "@[0-9]+" and "![0-9]+"

jdoerfert added a comment. (I'm going to sneak in `!range !` as well) Comment at: llvm/utils/UpdateTestChecks/common.py:330 +IR_VALUE_REGEXP_STRING += nameless_value.ir_prefix + r'(' + nameless_value.ir_regexp + r')' +IR_VALUE_REGEXP_SUFFIX = r'([,\s\(\)]|\Z)' +IR_VALUE_RE

[PATCH] D85735: [OpenMP] Context selector extensions for template functions

jdoerfert created this revision. jdoerfert added reviewers: JonChesterfield, jhuber6, ABataev. Herald added subscribers: llvm-commits, guansong, bollu, yaxunl. Herald added a reviewer: aaron.ballman. Herald added projects: clang, LLVM. jdoerfert requested review of this revision. Herald added a sub

[PATCH] D85735: [OpenMP] Context selector extensions for template functions

jdoerfert added a comment. In D85735#2210662 , @JonChesterfield wrote: > Nice. What makes it an extension? 5.0 / 2.3.5 claims "and where > variant-func-id is the name of a function variant that is either a base > language identifier or, for C++, a templ

[PATCH] D85777: [OpenMP] Support std::complex math functions in target regions

jdoerfert created this revision. jdoerfert added reviewers: JonChesterfield, jhuber6, ABataev. Herald added subscribers: guansong, bollu, yaxunl, mgorny. Herald added a project: clang. jdoerfert requested review of this revision. Herald added a subscriber: sstefan1. The last (big) missing piece to

[PATCH] D85777: [OpenMP] Support std::complex math functions in target regions

jdoerfert updated this revision to Diff 284898. jdoerfert added a comment. Update test and include mock `type_traits` header Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85777/new/ https://reviews.llvm.org/D85777 Files: clang/lib/Headers/CMakeL

[PATCH] D85777: [OpenMP] Support std::complex math functions in target regions

jdoerfert updated this revision to Diff 284954. jdoerfert added a comment. Reintroduce requires-target into tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85777/new/ https://reviews.llvm.org/D85777 Files: clang/lib/Headers/CMakeLists.txt

[PATCH] D85755: [OPENMP]Do not add TGT_OMP_TARGET_PARAM flag to non-captured mapped arguments.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. I don't follow this logic but that is not going to change right now. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85755/new/ https:/

[PATCH] D85777: [OpenMP] Support std::complex math functions in target regions

jdoerfert added a comment. I think the test fail because D85735 was not part of the build. Works locally just fine. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85777/new/ https://reviews.llvm.org/D85777 ___

[PATCH] D76211: OpenMP Metadirective with user defined condition

jdoerfert added a comment. Herald added subscribers: llvm-commits, sstefan1. Herald added a project: LLVM. Are you planning to add support for the actual OpenMP selectors as well? All the other traits, scoring, etc. I left a comment below that points you to the right place as we have almost all

[PATCH] D76211: OpenMP Metadirective with user defined condition

jdoerfert updated this revision to Diff 284961. jdoerfert added a comment. Update for @alokmishra.besu via arcanist as the webinterface doesn't allow patches over 2MB Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76211/new/ https://reviews.llvm.or

[PATCH] D84457: [OPENMP]Fix PR37671: Privatize local(private) variables in untied tasks.

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

[PATCH] D85099: [UpdateTestChecks] Match unnamed values like "@[0-9]+" and "![0-9]+"

This revision was automatically updated to reflect the committed changes. Closed by commit rG97ce7fd89fcc: [UpdateTestChecks] Match unnamed values like "@[0-9]+" and "![0-9]+" (authored by jdoerfert). Changed prior to commit: https://reviews.llvm.org/D85099?vs=284543&id=284972#toc Repository:

[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

jdoerfert added a comment. In D82317#2211643 , @guiand wrote: > After discussing with @eugenis, for the meantime it might be best to do the > following: > > - Change the masking attribute to be `-fdisable-noundef-analysis` (name > notwithstanding), and h

[PATCH] D85099: [UpdateTestChecks] Match unnamed values like "@[0-9]+" and "![0-9]+"

jdoerfert added inline comments. Comment at: llvm/utils/UpdateTestChecks/common.py:390 +for nameless_value in nameless_values: +if re.fullmatch(nameless_value.ir_prefix + r'[0-9]+?', var, re.IGNORECASE): +warn("Change IR value name '%s' to prevent possibl

[PATCH] D85875: [OpenMP][FIX] Do not crash trying to print a missing (demangled) user condition

jdoerfert created this revision. jdoerfert added reviewers: JonChesterfield, jhuber6, ABataev. Herald added subscribers: guansong, bollu, yaxunl. Herald added a project: clang. jdoerfert requested review of this revision. Herald added a subscriber: sstefan1. Repository: rG LLVM Github Monorepo

[PATCH] D85876: [OpenMP][FIX] Do not drop a '$' while demangling declare variant names

jdoerfert created this revision. jdoerfert added reviewers: JonChesterfield, jhuber6, ABataev. Herald added subscribers: guansong, bollu, yaxunl. Herald added a project: clang. jdoerfert requested review of this revision. Herald added a subscriber: sstefan1. Repository: rG LLVM Github Monorepo

[PATCH] D85877: [OpenMP] Support nested OpenMP context selectors (declare variant)

jdoerfert created this revision. jdoerfert added reviewers: JonChesterfield, jhuber6, ABataev. Herald added subscribers: guansong, bollu, yaxunl. Herald added a project: clang. jdoerfert requested review of this revision. Herald added a subscriber: sstefan1. Due to `omp begin/end declare variant`,

[PATCH] D85878: [OpenMP] Context selector extensions for return value overloading

jdoerfert created this revision. jdoerfert added reviewers: JonChesterfield, jhuber6, ABataev. Herald added subscribers: llvm-commits, guansong, bollu, kristof.beyls, yaxunl. Herald added a reviewer: aaron.ballman. Herald added projects: clang, LLVM. jdoerfert requested review of this revision. Her

[PATCH] D85879: [OpenMP] Overload `std::isnan` and friends multiple times for the GPU

jdoerfert created this revision. jdoerfert added reviewers: JonChesterfield, jhuber6, ABataev, MaskRay. Herald added subscribers: guansong, bollu, yaxunl. Herald added a project: clang. jdoerfert requested review of this revision. Herald added a subscriber: sstefan1. `std::isnan` and friends can b

[PATCH] D85735: [OpenMP] Context selector extensions for template functions

jdoerfert updated this revision to Diff 285264. jdoerfert added a comment. Rebase on top of D85878 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85735/new/ https://reviews.llvm.org/D85735 Files: clang/include/cl

[PATCH] D85879: [OpenMP] Overload `std::isnan` and friends multiple times for the GPU

jdoerfert updated this revision to Diff 285385. jdoerfert added a comment. Fix problem, add test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85879/new/ https://reviews.llvm.org/D85879 Files: clang/lib/Headers/__clang_cuda_cmath.h clang/test/

[PATCH] D85877: [OpenMP] Support nested OpenMP context selectors (declare variant)

jdoerfert updated this revision to Diff 285386. jdoerfert added a comment. Minor tweaks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85877/new/ https://reviews.llvm.org/D85877 Files: clang/include/clang/Basic/DiagnosticParseKinds.td clang/inc

[PATCH] D85879: [OpenMP] Overload `std::isnan` and friends multiple times for the GPU

jdoerfert added inline comments. Comment at: clang/lib/Headers/__clang_cuda_cmath.h:85 +//(note that we do not create implicit base functions here). To avoid +//this clash we add a new trait to some of them that is always true +//(this is LLVM after all ;)

[PATCH] D85878: [OpenMP] Context selector extensions for return value overloading

jdoerfert added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:5879 D.SetIdentifier(&VariantII, D.getBeginLoc()); return BaseFD; } ABataev wrote: > Does the user of this function expect that BaseFD can be `nullptr` after this > change? It was

[PATCH] D85875: [OpenMP][FIX] Do not crash trying to print a missing (demangled) user condition

jdoerfert added inline comments. Comment at: clang/lib/AST/OpenMPClause.cpp:2204-2207 +if (Selector.ScoreOrCondition) + Selector.ScoreOrCondition->printPretty(OS, nullptr, Policy); +else + OS << "..."; ABataev wrote: > Why it can

[PATCH] D85875: [OpenMP][FIX] Do not crash trying to print a missing (demangled) user condition

jdoerfert updated this revision to Diff 285463. jdoerfert added a comment. Remove leftover comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85875/new/ https://reviews.llvm.org/D85875 Files: clang/lib/AST/OpenMPClause.cpp clang/test/AST/as

[PATCH] D85879: [OpenMP] Overload `std::isnan` and friends multiple times for the GPU

jdoerfert added a comment. (I think the tests are failing due to missing parent revisions in the build, these have to go in in-order.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85879/new/ https://reviews.llvm.org/D85879 __

[PATCH] D85735: [OpenMP] Context selector extensions for template functions

jdoerfert updated this revision to Diff 285468. jdoerfert added a comment. Test formatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85735/new/ https://reviews.llvm.org/D85735 Files: clang/include/clang/Basic/AttrDocs.td clang/include/clan

[PATCH] D85938: [OpenMP][NFC] Update check lines after D85099

jdoerfert created this revision. jdoerfert added a reviewer: lebedev.ri. Herald added subscribers: guansong, bollu, yaxunl. Herald added a project: clang. jdoerfert requested review of this revision. Herald added a subscriber: sstefan1. Repository: rG LLVM Github Monorepo https://reviews.llvm.o

[PATCH] D85938: [OpenMP][NFC] Update check lines after D85099

jdoerfert planned changes to this revision. jdoerfert added a comment. Something is fishy here. Now the pre-merge tests fail... this should accept strictly more... I have to look into this, best guess: some non-determinism Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D85879: [OpenMP] Overload `std::isnan` and friends multiple times for the GPU

jdoerfert added inline comments. Comment at: clang/lib/Headers/__clang_cuda_cmath.h:85 +//(note that we do not create implicit base functions here). To avoid +//this clash we add a new trait to some of them that is always true +//(this is LLVM after all ;)

[PATCH] D85879: [OpenMP] Overload `std::isnan` and friends multiple times for the GPU

jdoerfert added inline comments. Comment at: clang/lib/Headers/__clang_cuda_cmath.h:85 +//(note that we do not create implicit base functions here). To avoid +//this clash we add a new trait to some of them that is always true +//(this is LLVM after all ;)

[PATCH] D85938: [OpenMP][NFC] Update check lines after D85099

jdoerfert added a comment. Root cause: We sometimes, depending on the invocation and potentially other factors, use a full path and sometimes not: So wen I run this with clang locally I get this: @98 = private unnamed_addr constant [98 x i8] c";/data/src/llvm-project/clang/test/OpenMP/irbuild

[PATCH] D85938: [OpenMP][NFC] Update check lines after D85099

jdoerfert updated this revision to Diff 285551. jdoerfert added a comment. This revision is now accepted and ready to land. Herald added subscribers: llvm-commits, ormris, hiraditya. Herald added a project: LLVM. Resolve the root cause Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D85878: [OpenMP] Context selector extensions for return value overloading

jdoerfert added a comment. In D85878#2217408 , @JonChesterfield wrote: > If I recall correctly, &foo with variants of foo returns a pointer to the > base. Correct. > If we have no base, and disable_implicit_base, what does &foo yield? It > should prob

[PATCH] D85938: [OpenMP][OMPIRBuilder] Use the source (=directory + filename) for locations

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9240e48a588c: [OpenMP][OMPIRBuilder] Use the source (=directory + filename) for locations (authored by jdoerfert). Changed prior to commit: https:

[PATCH] D86020: [MemCpyOptimizer] Optimize passing byref function arguments down the stack

jdoerfert added a comment. This needs an LLVM IR test. Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:1332 + if (CB->isByValArgument(i) || CB->paramHasAttr(i, Attribute::ByRef)) +MadeChange |= processByValOrByRefArgument(*CB, i); } --

[PATCH] D86037: [OpenMP][FIX] Do not use TBAA in type punning reduction GPU code PR46156

jdoerfert created this revision. jdoerfert added reviewers: JonChesterfield, ABataev. Herald added subscribers: guansong, bollu, kosarev, yaxunl. Herald added a project: clang. jdoerfert requested review of this revision. Herald added a subscriber: sstefan1. When we implement OpenMP GPU reductions

[PATCH] D85321: [OPENMP]Do not capture base pointer by reference if it is used as a base for array-like reduction.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. While we should be able to optimize this later in the pipeline soon, I guess we can do it early for now. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D86037: [OpenMP][FIX] Do not use TBAA in type punning reduction GPU code PR46156

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG95a25e4c3203: [OpenMP][FIX] Do not use TBAA in type punning reduction GPU code PR46156 (authored by jdoerfert). Changed prior to commit: https://r

[PATCH] D86020: [MemCpyOptimizer] Optimize passing byref function arguments down the stack

jdoerfert added a comment. This looks reasonable on first glance. @arsenm ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86020/new/ https://reviews.llvm.org/D86020 ___ cfe-commits mailing list cfe-commi

[PATCH] D86164: [OPENMP]Fix PR47158, case 2: do not report host-only functions in unused function in device mode.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM, nit: Is there a "not" missing in the the commit message? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86164/new/ https://reviews.ll

[PATCH] D74387: [SYCL] Defer __float128 type usage diagnostics

jdoerfert added a comment. In D74387#1992682 , @Fznamznon wrote: > Okay, seems like OpenMP needs unsupported types diagnosing as well. I'm > trying to adapt this patch for OpenMP, but it doesn't work out of the box > because it diagnoses memcpy like oper

[PATCH] D79186: [OPENMP]Consider 'omp_null_allocator' as a predefined allocator.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM. In D79186#2016893 , @ABataev wrote: > In D79186#2016737 , @jdoerfert wrote: > > > In D79186#201368

[PATCH] D74387: [SYCL] Defer __float128 type usage diagnostics

jdoerfert added a comment. In D74387#2026844 , @Fznamznon wrote: > I'm not sure that I've discovered all examples and problems, but I have a > couple of ones. I started with adapting current implementation for OpenMP and > right now I'm analyzing corresp

[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

jdoerfert created this revision. jdoerfert added reviewers: arsenm, spatel, efriedma, lebedev.ri, fhahn, rnk, hfinkel, regehr, nlopes. Herald added subscribers: dmgreen, bollu, wdng. Herald added a project: LLVM. After the discussion in D79454 it seemed clear we

[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

jdoerfert added a comment. > "Attributes on the call site argument and function argument are associated > with the original and copied memory respectively" > > This seems to fly in the face of existing practice, which is that function > attributes are copied to each callsite. I'd strongly prefe

[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

jdoerfert updated this revision to Diff 263024. jdoerfert marked 3 inline comments as done. jdoerfert added a comment. Fix spelling Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79636/new/ https://reviews.llvm.org/D79636 Files: llvm/docs/LangRef

[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

jdoerfert updated this revision to Diff 263026. jdoerfert added a comment. Minor wording tweak Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79636/new/ https://reviews.llvm.org/D79636 Files: llvm/docs/LangRef.rst Index: llvm/docs/LangRef.rst =

[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

jdoerfert added a comment. In D79636#2028294 , @efriedma wrote: > > Do you object to say that the call site argument and the argument point to > > distinct memory locations or something else? > > Like I said, my issue is with the "Attributes on the call s

[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

jdoerfert marked an inline comment as done. jdoerfert added inline comments. Comment at: llvm/docs/LangRef.rst:1052-1053 +memory of the callee. That means, a callee can write a ``byval`` parameter +and still be ``readonly`` or ``readnone`` because the write is, similar to

[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

jdoerfert added a comment. In D79636#2028479 , @efriedma wrote: > > @efriedma I'm a bit confused. Could you propose some wording so I get a > > feeling where you want to go? > > Depending on which direction we go, either: > > - "Attributes on a function o

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

jdoerfert added a comment. Thanks for these! We should probably split this in three patches. I commented below, mostly minor stuff. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPConstants.h:101 +extern Type *IntPtrTy; +extern Type *SizeTy; + I can totally

[PATCH] D79754: [OpenMP][AMDGCN] Support OpenMP offloading for AMDGCN architecture - Part 1

jdoerfert added inline comments. Comment at: clang/lib/AST/Decl.cpp:3224 + if (Context.getTargetInfo().getTriple().isAMDGCN() && + Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID) && sameerds wrote: > arsenm wrote: > > This is also identical to the

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