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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGac892c70a456: [OMPIRBuilder] Add support for simdlen clause (authored by psoni2628, committed by arnamoy10). Repository: rG LLVM Github Monorepo

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

psoni2628 updated this revision to Diff 443622. psoni2628 added a comment. - Rebase, ensuring that build bot failures are gone CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129149/new/ https://reviews.llvm.org/D129149 Files: clang/lib/CodeGen/CGStmtOpenMP.cpp clang/test/OpenMP/irbu

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

Meinersbur accepted this revision. Meinersbur added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129149/new/ https://reviews.llvm.org/D129149 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-

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

psoni2628 updated this revision to Diff 442715. psoni2628 marked 4 inline comments as done. psoni2628 added a comment. Herald added subscribers: bzcheeseman, awarzynski, sdasgup3, wenzhicui, wrengr, dcaballe, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, s

[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] D129149: [OMPIRBuilder] Add support for simdlen clause

Meinersbur added a comment. 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 `unrollLoopPartial` it does not make sense to have the unroll factor set by a different method. It would

[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

psoni2628 marked 8 inline comments as done. psoni2628 added inline comments. Comment at: clang/test/OpenMP/irbuilder_simdlen.cpp:1 +// RUN: %clang_cc1 -no-opaque-pointers -fopenmp-enable-irbuilder -verify -fopenmp -fopenmp-version=45 -x c++ -triple x86_64-unknown-unknown -emit-l

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

psoni2628 updated this revision to Diff 442637. psoni2628 added a comment. - Autogenerate check lines for test case - Use isa instead of dyncast - Remove unused DebugLoc - Remove `llvm::` from `llvm::ConstantInt` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129149/new/ https://reviews.

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

psoni2628 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. jdoerfert wrote: > psoni2628 wrote: > > jdoerfe

[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] D129149: [OMPIRBuilder] Add support for simdlen clause

psoni2628 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. jdoerfert wrote: > No debug location needed. Yo

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

psoni2628 added inline comments. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2885 +void OpenMPIRBuilder::applySimdlen(DebugLoc, CanonicalLoopInfo *CanonicalLoop, + llvm::ConstantInt *Simdlen) { + LLVMContext &Ctx = Builder.getContext()

[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

psoni2628 updated this revision to Diff 442595. psoni2628 added a comment. - Remove discouraged else after return CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129149/new/ https://reviews.llvm.org/D129149 Files: clang/lib/CodeGen/CGStmtOpenMP.cpp clang/test/OpenMP/irbuilder_simd.cp

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

kiranchandramohan added a comment. In D129149#3632861 , @psoni2628 wrote: > In D129149#3632813 , > @kiranchandramohan wrote: > >> Nit: Also add to the summary that this patch uses the simdlen support in >> OpenM

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

psoni2628 added a comment. In D129149#3632813 , @kiranchandramohan wrote: > Nit: Also add to the summary that this patch uses the simdlen support in > OpenMPIRBuilder when it is enabled in Clang. I just wanted to clarify that below is what you mean?

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

kiranchandramohan added a comment. Nit: Also add to the summary that this patch uses the simdlen support in OpenMPIRBuilder when it is enabled in Clang. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2600 + continue; +else + return false; Nit: El

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

psoni2628 added inline comments. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2596 // Check for unsupported clauses - if (!S.clauses().empty()) { -// Currently no clause is supported -return false; + for (OMPClause *C : S.clauses()) { +// Currently only simdlen

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

psoni2628 updated this revision to Diff 442555. psoni2628 marked 3 inline comments as done. psoni2628 added a comment. - Create new tests instead of modifying existing ones - Specialize `isSupportedByOpenMPIRBuilder` for `OMPSimdDirective` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12

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

shraiysh added inline comments. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2596 // Check for unsupported clauses - if (!S.clauses().empty()) { -// Currently no clause is supported -return false; + for (OMPClause *C : S.clauses()) { +// Currently only simdlen

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

psoni2628 added inline comments. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2596 // Check for unsupported clauses - if (!S.clauses().empty()) { -// Currently no clause is supported -return false; + for (OMPClause *C : S.clauses()) { +// Currently only simdlen

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

shraiysh added inline comments. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2596 // Check for unsupported clauses - if (!S.clauses().empty()) { -// Currently no clause is supported -return false; + for (OMPClause *C : S.clauses()) { +// Currently only simdlen

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

domada added inline comments. Comment at: clang/test/OpenMP/irbuilder_simd.cpp:15 P pp; -#pragma omp simd +#pragma omp simd simdlen(3) for (int i = 3; i < 32; i += 5) { Could you add separate test case instead of modifying existing test case? Repository:

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

psoni2628 marked an inline comment as not done. psoni2628 added inline comments. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2596 // Check for unsupported clauses - if (!S.clauses().empty()) { -// Currently no clause is supported -return false; + for (OMPClause *C

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

psoni2628 added inline comments. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2596 // Check for unsupported clauses - if (!S.clauses().empty()) { -// Currently no clause is supported -return false; + for (OMPClause *C : S.clauses()) { +// Currently only simdlen

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

arnamoy10 added inline comments. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2596 // Check for unsupported clauses - if (!S.clauses().empty()) { -// Currently no clause is supported -return false; + for (OMPClause *C : S.clauses()) { +// Currently only simdlen