[PATCH] D82659: Fix missing build dependency on omp_gen.

clementval accepted this revision. clementval added a comment. LGTM. So later the DEPENDS omp_gen that are in clang subdirectories could be removed right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82659/new/ https://reviews.llvm.org/D82659

[PATCH] D82659: Fix missing build dependency on omp_gen.

clementval added a comment. @simon_tatham Can you just update the description so it is consistent with your fix when it lands? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82659/new/ https://reviews.llvm.org/D82659

[PATCH] D81098: [OpenMP] Upgrade default version of OpenMP to 5.0

clementval added a comment. In D81098#2112159 , @ABataev wrote: > LG Since this revision landed two tests are failing. ` libomp.env::kmp_set_dispatch_buf.c` and `libomp.worksharing/for::kmp_set_dispatch_buf.c`. It was also reported on the mailing

[PATCH] D82659: Fix missing build dependency on omp_gen.

clementval added a comment. In D82659#2138228 , @michele.scandale wrote: > In D82659#2136999 , @clementval > wrote: > > > Looks good but just one question ... When clang is built as standalone it > > does not

[PATCH] D82659: Fix missing build dependency on omp_gen.

clementval added a comment. In D82659#2130022 , @simon_tatham wrote: > @jdoerfert , @clementval : over in D83032 > is a polished-up version of the script I used to check where the missing deps > needed to go. Might be

[PATCH] D82659: Fix missing build dependency on omp_gen.

clementval added a comment. In D82659#2136909 , @michele.scandale wrote: > Why `omp_gen` is now a dependency of `clang-tablegen-targets` rather than > being in the `LLVM_COMMON_DEPENDS` list like `clang-tablegen-targets`? > > Moreover I've noticed that

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

clementval added a comment. In D81736#2096336 , @jdoerfert wrote: > In D81736#2095947 , @clementval > wrote: > > > In D81736#2093926 , @jdoerfert > > wrote: > > > > >

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

clementval marked 10 inline comments as done. clementval added a comment. @jdenny I will have a look at your suggestion to dump the name instead of the id. If it looks straight forward I will do it. Other comments were addressed. Thanks. Comment at:

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

clementval reopened this revision. clementval added a comment. This revision is now accepted and ready to land. Missing dependencies Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81736/new/ https://reviews.llvm.org/D81736

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

clementval added a comment. In D81736#2107202 , @jdenny wrote: > My cmake skills are lacking. Why are there are so many new DEPENDS > relationships where there were none before? Is it because omp_gen is > generating a header file that's included

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

clementval requested review of this revision. clementval added a comment. @jdoerfert @jdenny I had to add a bunch of dependencies so that the file is generated correctly when needed. Do you have any feedback on this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

clementval added a comment. @jdoerfert @jdenny Should we wait until Monday to go ahead with this patch? I have several other patches that will follow this one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81736/new/

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

clementval marked 2 inline comments as done. clementval added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/CMakeLists.txt:2 +set(LLVM_TARGET_DEFINITIONS OMP.td) +tablegen(LLVM OMP.h.inc --gen-directive-decls) +add_public_tablegen_target(omp_gen)

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

clementval marked 3 inline comments as done. clementval added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/CMakeLists.txt:2 +set(LLVM_TARGET_DEFINITIONS OMP.td) +tablegen(LLVM OMP.h.inc --gen-directive-decls) +add_public_tablegen_target(omp_gen)

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

clementval created this revision. clementval added reviewers: vdmitrie, jdoerfert, jdenny. Herald added subscribers: cfe-commits, sstefan1, guansong, yaxunl. Herald added a project: clang. Previously OMPD_unknown was last item in the Directive enumeration and its position was used in various

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

clementval marked 3 inline comments as done. clementval added inline comments. Comment at: clang/lib/Parse/ParseOpenMP.cpp:197 } return DKind < OMPD_unknown ? static_cast(DKind) : OMPD_unknown; vdmitrie wrote: > Should this

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

clementval updated this revision to Diff 273216. clementval added a comment. add unsigned cast Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82518/new/ https://reviews.llvm.org/D82518 Files: clang/lib/Basic/OpenMPKinds.cpp

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

clementval marked an inline comment as done. clementval added a comment. In D81736#2117858 , @thakis wrote: > This seems to cause build errors sometimes when buildling libclang. Maybe > there are more missing dependencies? Example failure: >

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

clementval added a comment. In D81736#2093926 , @jdoerfert wrote: > Assuming this passes all the tests, it looks good to me. Let's wait a day or > two for people to take a look though :) @jdoerfert Sure let's wait a bit. I see a failure because of some

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

clementval marked 3 inline comments as done. clementval added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMP.td:496 +def OMP_EndDeclareVariant : Directive<"end declare variant"> {} +def OMP_Unknown : Directive<"unknown"> {} jdoerfert wrote: >

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG5b9ce07a761f: [openmp] Use Directive_enumSize instead of OMPD_unknown position (authored by clementval). Changed prior to commit: https://reviews.llvm.org/D82518?vs=273216=273327#toc Repository: rG

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

clementval marked 2 inline comments as done. clementval added inline comments. Comment at: clang/lib/Basic/OpenMPKinds.cpp:583 OpenMPDirectiveKind DKind) { - assert(DKind <= OMPD_unknown); + assert(unsigned(DKind) <= llvm::omp::Directive_enumSize); switch (DKind) {

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

clementval updated this revision to Diff 270481. clementval added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Keep same ordering than OMPKinds.def + add dependency Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D94261: [clang] Add powerpc64le-none-linux-gnu to gnu toolchain for PPC64

clementval created this revision. clementval added reviewers: dlj, jdenny. Herald added subscribers: steven.zhang, shchenz, nemanjai. clementval requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang.

[PATCH] D94261: [clang] Add powerpc64le-none-linux-gnu to gnu toolchain for PPC64

This revision was automatically updated to reflect the committed changes. Closed by commit rG48baa7f5b11c: [clang] Add powerpc64le-none-linux-gnu to gnu toolchain for PPC64 (authored by clementval). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D92955: [openmp] Remove clause from OMPKinds.def and use OMP.td info

clementval updated this revision to Diff 310589. clementval added a comment. Remove useless define macro Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92955/new/ https://reviews.llvm.org/D92955 Files: clang/include/clang/AST/ASTFwd.h

[PATCH] D92955: [openmp] Remove clause from OMPKinds.def and use OMP.td info

clementval created this revision. clementval added reviewers: jdoerfert, jdenny, kiranchandramohan. Herald added subscribers: jfb, arphaman, guansong, yaxunl. clementval requested review of this revision. Herald added subscribers: llvm-commits, cfe-commits, sstefan1. Herald added projects: clang,

[PATCH] D92955: [openmp] Remove clause from OMPKinds.def and use OMP.td info

clementval added a comment. In D92955#2443313 , @jdoerfert wrote: > I like this a lot. We might even look into generating the clang OpenMPClause > classes via TableGen later, thanks a lot. > > There is a file missing, right? The list of actual clauses.

[PATCH] D92955: [openmp] Remove clause from OMPKinds.def and use OMP.td info

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa7b2847216b4: [openmp] Remove clause from OMPKinds.def and use OMP.td info (authored by clementval). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D92955: [openmp] Remove clause from OMPKinds.def and use OMP.td info

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf4c8b8031800: [openmp] Remove clause from OMPKinds.def and use OMP.td info (authored by clementval). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D92955: [openmp] Remove clause from OMPKinds.def and use OMP.td info

clementval updated this revision to Diff 310980. clementval added a comment. Herald added a subscriber: mgorny. Fix -Werror=covered-switch-default problems + rename OMP.cpp.inc to OMP.inc since the .cpp does not make sense anymore. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D93301: [flang][driver] Add support for `-c` and `-emit-obj`

clementval added inline comments. Comment at: clang/include/clang/Driver/Options.td:4629 -} // let Flags = [CC1Option] +} // let Flags = [CC1Option, NoDriverOption] + Is it intended to modify the SYCL options? Comment at:

[PATCH] D92955: [openmp] Remove clause from OMPKinds.def and use OMP.td info

clementval added a comment. Ping since there was a phab email delivery problem. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92955/new/ https://reviews.llvm.org/D92955 ___ cfe-commits mailing list

[PATCH] D93301: [flang][driver] Add support for `-c` and `-emit-obj`

clementval accepted this revision. clementval 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/D93301/new/ https://reviews.llvm.org/D93301

[PATCH] D92955: [openmp] Remove clause from OMPKinds.def and use OMP.td info

clementval added a comment. In D92955#2445815 , @thakis wrote: > And a less philosophical comment: This causes a bunch of > Wcovered-switch-default warnings: > > ../../clang/include/clang/AST/OpenMPClause.h::5: warning: default label > in switch

[PATCH] D101503: [OpenMPIRBuilder] Add createOffloadMaptypes and createOffloadMapnames functions

clementval updated this revision to Diff 341589. clementval added a comment. Fix global variable names. Separator comes from OpenMP runtime information. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101503/new/ https://reviews.llvm.org/D101503

[PATCH] D101503: [OpenMPIRBuilder] Add createOffloadMaptypes and createOffloadMapnames functions

clementval updated this revision to Diff 341938. clementval marked 5 inline comments as done. clementval added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101503/new/ https://reviews.llvm.org/D101503 Files:

[PATCH] D101503: [OpenMPIRBuilder] Add createOffloadMaptypes and createOffloadMapnames functions

clementval added a comment. In D101503#2727906 , @Meinersbur wrote: > testcase > > > That is, something that calls

[PATCH] D101503: [OpenMPIRBuilder] Add createOffloadMaptypes and createOffloadMapnames functions

clementval created this revision. clementval added reviewers: jdoerfert, kiranchandramohan. Herald added subscribers: rriddle, guansong, hiraditya, yaxunl. clementval requested review of this revision. Herald added subscribers: llvm-commits, cfe-commits, sstefan1, stephenneuendorffer. Herald

[PATCH] D101503: [OpenMPIRBuilder] Add createOffloadMaptypes and createOffloadMapnames functions

clementval updated this revision to Diff 342508. clementval added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101503/new/ https://reviews.llvm.org/D101503 Files: clang/lib/CodeGen/CGOpenMPRuntime.cpp

[PATCH] D101503: [OpenMPIRBuilder] Add createOffloadMaptypes and createOffloadMapnames functions

clementval updated this revision to Diff 342512. clementval added a comment. Rebase 2 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101503/new/ https://reviews.llvm.org/D101503 Files: clang/lib/CodeGen/CGOpenMPRuntime.cpp

[PATCH] D101503: [OpenMPIRBuilder] Add createOffloadMaptypes and createOffloadMapnames functions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG63f8226f259a: [OpenMPIRBuilder] Add createOffloadMaptypes and createOffloadMapnames functions (authored by clementval). Repository: rG LLVM

[PATCH] D76342: [OpenMP] Implement '#pragma omp tile'

clementval added a comment. A fix is ready hereL https://reviews.llvm.org/D96808 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76342/new/ https://reviews.llvm.org/D76342 ___ cfe-commits mailing list

[PATCH] D76342: [OpenMP] Implement '#pragma omp tile'

clementval added a comment. @Meinersbur This patch is making Flang buildbots failing. Since you have added a clause in `OMP.td` you have to add lines here as well ->

[PATCH] D121198: [flang][driver] Add support for `-debug-dump-pft`

clementval added a comment. I’m not sure this is a feature we want to expose in the main driver. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121198/new/ https://reviews.llvm.org/D121198 ___

[PATCH] D121198: [flang][driver] Add support for `-debug-dump-pft`

clementval accepted this revision. clementval added a comment. In D121198#3366774 , @awarzynski wrote: > In D121198#3366741 , @clementval > wrote: > >> I’m not sure this is a feature we want to expose in the

[PATCH] D122008: [flang][driver] Add support for generating executables

clementval added a comment. In D122008#3426847 , @awarzynski wrote: > We discussed this patch in our community call today and some people expressed > their reservations about merging this just now. As I mentioned, I would like > to have this merged to

[PATCH] D93401: [flang][driver] Add support for `-D`, `-U`

clementval added inline comments. Comment at: flang/include/flang/Frontend/CompilerInvocation.h:89 + /// These values are found in the preprocessor options. + void setFortranOpts(); }; @awarzynski I was looking at this file recently and I was wondering why we

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

clementval added a comment. @pengfei We are also hitting the following assertion with this patch. Do you have any idea why? /llvm-project/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:4333: void {anonymous}::SelectionDAGLegalize::ConvertNodeToLibcall(llvm::SDNode*): Assertion

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

clementval added inline comments. Comment at: clang/lib/Testing/CMakeLists.txt:30 + clangBasic + clangFrontend ) You still have these changes that are unrelated. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D125788: [flang][driver] Rename `flang-new` as `flang`

clementval added inline comments. Comment at: clang/lib/Driver/ToolChain.cpp:185 {"flang", "--driver-mode=flang"}, + {"flang-new", "--driver-mode=flang"}, {"clang-dxc", "--driver-mode=dxc"}, Why do we need two lines here? Shouldn't we have a

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

clementval added a comment. In D105584#3617456 , @abidmalikwaterloo wrote: > // CHECK-LABEL: omp_DistributeOp > func.func @omp_DistributeOp(%lb : index, %ub : index, %step : index, > %data_var : memref, %chunk_var : i32) -> () { > // CHECK:

[PATCH] D105255: [MLIR][OpenMP] Added target data, exit data, and enter data operation definition for MLIR.

clementval added a comment. Why not adding the assemblyFormat directly for these operations like your patch D105584 ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105255/new/ https://reviews.llvm.org/D105255

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

clementval added inline comments. Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:449 ParentOneOf<["WsLoopOp", "ReductionDeclareOp", - "AtomicUpdateOp", "SimdLoopOp"]>]> { + "AtomicUpdateOp", "SimdLoopOp","DistributeOp"]>]> { let summary = "loop yield

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

clementval added inline comments. Comment at: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp:215 +LogicalResult DistributeOp::verify(){ + if (this->lowerBound().empty()) { +return emitOpError() << "empty lowerbound for distribute loop operation"; No brace -

[PATCH] D125788: [flang][driver] Rename `flang-new` as `flang`

clementval requested changes to this revision. clementval added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Driver/ToolChain.cpp:185 {"flang", "--driver-mode=flang"}, + {"flang-new", "--driver-mode=flang"},

[PATCH] D125788: [flang][driver] Rename `flang-new` as `flang`

clementval added a comment. In D125788#3621744 , @awarzynski wrote: > We discussed this in our call on Monday and agreed to go ahead provided that > this change is technically sound. IIUC, this has now been confirmed: > >> Overall the patch looks ok

[PATCH] D125788: [flang][driver] Rename `flang-new` as `flang`

clementval added inline comments. Comment at: clang/lib/Driver/ToolChain.cpp:185 {"flang", "--driver-mode=flang"}, + {"flang-new", "--driver-mode=flang"}, {"clang-dxc", "--driver-mode=dxc"}, awarzynski wrote: > richard.barton.arm wrote: > >

[PATCH] D125788: [flang][driver] Rename `flang-new` as `flang`

clementval added a comment. Overall the patch looks ok from a technical point. Shouldn't we just wait until we can make the permanent renaming so we do not add unnecessary cmake option? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125788/new/

[PATCH] D122008: [flang][driver] Add support for generating executables

clementval added a comment. Do you plan to discuss this again at during the next call? Note that today is a holiday in various country in Europe (maybe elsewhere too) so the one on 4/27 is probably better. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D130822: Fixed loads of typos

clementval requested changes to this revision. clementval added inline comments. This revision now requires changes to proceed. Comment at: flang/docs/Extensions.md:157 with each other. -* Values for whole anonymous parent components in structure constructors (e.g.,

[PATCH] D131915: [MLIR][OpenMP] Added target data, exit data, and enter data operation definition for MLIR.

clementval added inline comments. Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:828 +(`use_device_addr` `(` $use_device_addr^ `:` type($use_device_addr) `)` )? +(`map` `(` $map_type_modifier_val `,` $map_type_val `:` $map_operands^ `:` type($map_operands)

[PATCH] D129156: Add -fpass-plugin option to Flang

clementval added a comment. I get a failure since this commit. `Could not load library By.so` ... `cannot open shared object file: No such file or directory`. Maybe a missing dependency? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129156/new/

[PATCH] D129156: Add -fpass-plugin option to Flang

clementval added a comment. In D129156#3836768 , @tarunprabhu wrote: > I think there is a typo somehow. It should be "Bye.so". Not sure how that > happened. I'm looking into this. I probably didn't copy the error correctly .. this is the correct libs

[PATCH] D129156: Add -fpass-plugin option to Flang

clementval added a comment. As @awarzynski mentioned, it is likely a missing dependency on the test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129156/new/ https://reviews.llvm.org/D129156 ___

[PATCH] D129156: Add -fpass-plugin option to Flang

clementval added a comment. In D129156#3848692 , @awarzynski wrote: > In D129156#3847396 , @tarunprabhu > wrote: > >> Added `examples` to `REQUIRES` in `test/Driver/pass-plugin.f90. > > Thanks for the update! >

[PATCH] D105255: [MLIR][OpenMP] Added target data, exit data, and enter data operation definition for MLIR.

clementval added a comment. In D105255#3723865 , @abidmalikwaterloo wrote: > [MLIR][OpenMP] Added target data, exit data, and enter data operation > definition for MLIR Looks like it's not the complete revision. There are almost no diffs anymore.

[PATCH] D105255: [MLIR][OpenMP] Added target data, exit data, and enter data operation definition for MLIR.

clementval added a comment. In D105255#3707495 , @raghavendhra wrote: > In D105255#3707088 , > @abidmalikwaterloo wrote: > >> In D105255#3706602 , @raghavendhra >>

[PATCH] D105255: [MLIR][OpenMP] Added target data, exit data, and enter data operation definition for MLIR.

clementval added a comment. In D105255#3708258 , @abidmalikwaterloo wrote: > In D105255#3707533 , @clementval > wrote: > >> In D105255#3707495 , @raghavendhra >>

[PATCH] D140795: [Flang] Add user option -funderscoring/-fnounderscoring to enable/disable ExternalNameConversionPass

clementval requested changes to this revision. clementval added a comment. This revision now requires changes to proceed. You need to update the driver-help tests. Comment at: flang/include/flang/Tools/CLOptions.inc:194 #if !defined(FLANG_EXCLUDE_CODEGEN) -inline void

[PATCH] D140795: [Flang] Add user option -funderscoring/-fnounderscoring to enable/disable ExternalNameConversionPass

clementval added a comment. You should have a look at https://mlir.llvm.org/docs/PassManagement/#instance-specific-pass-options Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140795/new/ https://reviews.llvm.org/D140795

[PATCH] D138675: [flang] Add -ffast-math and -Ofast

clementval added a comment. Herald added a subscriber: jdoerfert. I have the `Driver/fast_math.f90` failing on my side. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138675/new/ https://reviews.llvm.org/D138675

[PATCH] D138675: [flang] Add -ffast-math and -Ofast

clementval added a comment. In D138675#3988110 , @tblah wrote: > In D138675#3987836 , @clementval > wrote: > >> I have the `Driver/fast_math.f90` failing on my side. >> >>

[PATCH] D131915: [MLIR][OpenMP] Added target data, exit data, and enter data operation definition for MLIR.

clementval added inline comments. Comment at: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp:584 +std::stringstream typeMod, type; +if (mapTypeBits & 0x04) + typeMod << "always "; jdoerfert wrote: > TIFitis wrote: > > kiranchandramohan wrote: > > >

[PATCH] D131915: [MLIR][OpenMP] Added target data, exit data, and enter data operation definition for MLIR.

clementval added a comment. Just small nit Comment at: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp:628-629 +int64_t mapTypeBits = 0x00; +auto mapOp = map_operands[i]; +auto mapTypeOp = map_types[i]; + auto should be spelled out here.

[PATCH] D140795: [Flang] Add user option -funderscoring/-fnounderscoring to control trailing underscore added to external names

clementval added a comment. Small suggestion Comment at: flang/lib/Optimizer/Transforms/ExternalNameConversion.cpp:41-45 + std::string newName{result.second.name}; + if (appendUnderscore) +newName = newName + "_"; + + return newName; To avoid new copy

[PATCH] D140795: [Flang] Add user option -funderscoring/-fnounderscoring to control trailing underscore added to external names

clementval added a comment. Can you also add test in `flang/test/Fir/external-mangling.fir`? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140795/new/ https://reviews.llvm.org/D140795 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

clementval added inline comments. Comment at: mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1342-1382 +/// Create a constant string location from the MLIR Location information. +static llvm::Constant * +createSourceLocStrFromLocation(Location loc,

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

clementval added inline comments. Comment at: mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1342-1382 +/// Create a constant string location from the MLIR Location information. +static llvm::Constant * +createSourceLocStrFromLocation(Location loc,

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

clementval added inline comments. Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:449 ParentOneOf<["WsLoopOp", "ReductionDeclareOp", - "AtomicUpdateOp", "SimdLoopOp"]>]> { + "AtomicUpdateOp", "SimdLoopOp","DistributeOp"]>]> { let summary = "loop yield

[PATCH] D137329: [flang] Add -f[no-]associative-math and -mreassociate

clementval added a comment. Herald added a subscriber: jdoerfert. Wouldn't it be good to have a RFC for all these options and what they will do in Flang instead of just adding them all? Or did I miss the RFC? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

clementval added a comment. I still see some review comments not done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105584/new/ https://reviews.llvm.org/D105584 ___ cfe-commits mailing list

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

clementval added a comment. Herald added a reviewer: nicolasvasilache. Herald added subscribers: Moerafaat, zero9178. Are you still working on this? Otherwise please close it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105584/new/

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

clementval added inline comments. Comment at: mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1342-1382 +/// Create a constant string location from the MLIR Location information. +static llvm::Constant * +createSourceLocStrFromLocation(Location loc,

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

clementval added inline comments. Comment at: mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1342-1382 +/// Create a constant string location from the MLIR Location information. +static llvm::Constant * +createSourceLocStrFromLocation(Location loc,

[PATCH] D143704: [flang] Feature list plugin

clementval added a comment. You have some failed buildbots. Can you revert it if you don't have a quick fix? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143704/new/ https://reviews.llvm.org/D143704

[PATCH] D143704: [flang] Feature list plugin

clementval added a comment. In D143704#4202146 , @elmcdonough wrote: > Haven't been able to check this yet, but I think it might be due to the tests > expecting a deterministic output. I've only ever tested on Ubuntu WSL, so > its possible the order

[PATCH] D143704: [flang] Feature list plugin

clementval added a comment. I reverted your patch since the buildbot was failing for couple of hours. Feel free to re-land it when you have a fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143704/new/ https://reviews.llvm.org/D143704

[PATCH] D143704: [flang] Feature list plugin

clementval added a comment. pre-commit ci is showing a different failure: TEST 'Flang :: Examples/feature-list-class.f90' FAILED Script: -- : 'RUN: at line 4'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/flang-new -fc1 -load

[PATCH] D143704: [flang] Feature list plugin

clementval added a comment. The pre-commit was failing. Please do not commit without at least checking that the pre-commit is fine. https://buildkite.com/llvm-project/premerge-checks/builds/142487#018707c0-3ec0-4daa-a3a4-ffc815807994 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D145845: [Flang] Allow compile *.f03, *.f08 file

clementval added a comment. Thanks. Can you add tests in `flang/test/Driver`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145845/new/ https://reviews.llvm.org/D145845 ___ cfe-commits mailing list

[PATCH] D145845: [Flang] Allow compile *.f03, *.f08 file

clementval added a comment. In D145845#4186607 , @awarzynski wrote: > Thanks! > > In D145845#4186590 , @clementval > wrote: > >> Thanks. Can you add tests in `flang/test/Driver`? > > Hm, this is a `clangDriver`

[PATCH] D140795: [Flang] Add user option -funderscoring/-fnounderscoring to control trailing underscore added to external names

clementval accepted this revision. clementval added a comment. This revision is now accepted and ready to land. LGTM. Thanks for working on this. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140795/new/ https://reviews.llvm.org/D140795 ___

[PATCH] D140795: [Flang] Add user option -funderscoring/-fnounderscoring to control trailing underscore added to external names

clementval added a comment. You still have style mismatch in `flang/include/flang/Tools/CLOptions.inc`. This file is likely not run through clang-format so that's why. Comment at: flang/include/flang/Tools/CLOptions.inc:214 +llvm::OptimizationLevel optLevel =

[PATCH] D143493: [flang][driver] Add support for -include flag in flang -fc1

clementval added a comment. In D143493#4110230 , @awarzynski wrote: > In what cases would this flag be used in practice? I've scanned Clang and > couldn't find any answers. +1 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D140795: [Flang] Add user option -funderscoring/-fnounderscoring to control trailing underscore added to external names

clementval added inline comments. Comment at: flang/test/Fir/external-mangling.fir:36 + +// CHECK-NOUNDER: func @foo +// CHECK-NOUNDER: %{{.*}} = fir.address_of(@a) : !fir.ref> You should check at least the character after it because here the check line would

[PATCH] D140795: [Flang] Add user option -funderscoring/-fnounderscoring to control trailing underscore added to external names

clementval added a comment. You still have clang-format issues in your patch. Can you update that. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140795/new/ https://reviews.llvm.org/D140795 ___ cfe-commits mailing list

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

clementval added inline comments. Comment at: mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1342-1382 +/// Create a constant string location from the MLIR Location information. +static llvm::Constant * +createSourceLocStrFromLocation(Location loc,

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

clementval added inline comments. Comment at: mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1342-1382 +/// Create a constant string location from the MLIR Location information. +static llvm::Constant * +createSourceLocStrFromLocation(Location loc,

[PATCH] D142244: [flang][driver] Add support for -embed-offload-object flag in flang -fc1

clementval added a comment. In D142244#4096648 , @jsjodin wrote: > In D142244#4096579 , @clementval > wrote: > >> In D142244#4096546 , @jsjodin >> wrote: >> >>> In

  1   2   >