[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

kiranchandramohan added a comment. In D147218#4645055 , @skatrak wrote: > Thank you for the review. After I address your last comment my plan is to > land this and the other two accepted REQUIRES patches that depended on it. > > Is there a preferred appr

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

kiranchandramohan accepted this revision. kiranchandramohan added a comment. This revision is now accepted and ready to land. LG. Comment at: flang/lib/Lower/Bridge.cpp:2366-2367 mlir::OpBuilder::InsertPoint insertPt = builder->saveInsertionPoint(); +analyzeOpenMPDecla

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

kiranchandramohan added a comment. Could you add to the summary that the `atomic` related handling is done elsewhere. Could you expand the tests to cover the various `if` conditions that are used in the code? Comment at: flang/include/flang/Lower/OpenMP.h:16 +#include "mli

[PATCH] D147219: [OpenMP][Flang][MLIR] Lowering of requires directive from MLIR to LLVM IR

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

[PATCH] D159212: [MLIR] Allow dialects to disable CSE for certain operations

kiranchandramohan added a comment. I think this requires an RFC in MLIR discourse to get some feedback from the experts. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159212/new/ https://reviews.llvm.org/D159212 __

[PATCH] D158430: [flang][driver] Mark -fuse-ld as visible in Flang

kiranchandramohan added a comment. @erjin Could you land this patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158430/new/ https://reviews.llvm.org/D158430 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D158507: [Flang][Driver] Add support for fomit-frame-pointer

kiranchandramohan added a comment. For our immediate purpose, I think changing the visibility in the Driver to include `flang` is sufficient. After that, we can spend time implementing this properly by refactoring the code to llvm. Comment at: llvm/include/llvm/Frontend/Drive

[PATCH] D158430: [flang][driver] Mark -fuse-ld as visible in Flang

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

[PATCH] D158309: [flang][driver] Mark -Wl as visible in Flang

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

[PATCH] D158289: [flang][driver] Partial revert of https://reviews.llvm.org/D157837

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

[PATCH] D147219: [OpenMP][Flang][MLIR] Lowering of requires directive from MLIR to LLVM IR

kiranchandramohan added inline comments. Comment at: mlir/lib/Target/LLVMIR/ModuleTranslation.cpp:1388-1393 + // Convert module itself before any functions and operations inside, so that + // the OpenMPIRBuilder is configured with the OpenMP dialect attributes + // attached to

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

kiranchandramohan added inline comments. Comment at: flang/lib/Frontend/CompilerInvocation.cpp:156-158 +/// Parse a remark command line argument. It may be missing, disabled/enabled by +/// '-R[no-]group' or specified with a regular expression by '-Rgroup=regexp'. +/// On top of

[PATCH] D157410: [Flang][Driver] Enable Rpass and other R family options.

This revision was automatically updated to reflect the committed changes. Closed by commit rG6e13e3c3e5e2: [Flang][Driver] Enable Rpass and other R family options. (authored by Victor Kingi , committed by kiranchandramohan). Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D157493: Revert "Revert "[Flang][Sema] Move directive sets to a shared location""

kiranchandramohan accepted this revision. kiranchandramohan added a comment. This revision is now accepted and ready to land. LG. Please wait for the CI to pass. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157493/new/ https://reviews.llvm.org/D15

[PATCH] D157410: [Flang][Driver] Enable Rpass and other R family options.

kiranchandramohan accepted this revision. kiranchandramohan added a comment. LG. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157410/new/ https://reviews.llvm.org/D157410 ___ cfe-commits mailing list cf

[PATCH] D157090: [Flang][Sema] Move directive sets to a shared location

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

[PATCH] D157090: [Flang][Sema] Move directive sets to a shared location

kiranchandramohan added a comment. This looks OK. The only concern is whether we will lose the ability to inline the code for set membership. Can these sets be put in a header file with `inline constexpr`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D156320: [FLang] Add support for Rpass flag

kiranchandramohan added a comment. > rpass flag now prints remarks when requested but does not display > the passName used, i.e [-Rpass=inline] I think the location information is also not printed. Please check the difference in implementation of the `TextDiagnosticPrinter::HandleDiagnostic` fu

[PATCH] D156524: [flang][nfc] Simplify option forwarding

kiranchandramohan accepted this revision. kiranchandramohan added a comment. This revision is now accepted and ready to land. LG. Comment at: clang/lib/Driver/ToolChains/Flang.cpp:146-147 + + Args.AddAllArgs(CmdArgs, {options::OPT_flang_experimental_hlfir, +

[PATCH] D155452: [Flang] Add support for fsave-optimization-record

This revision was automatically updated to reflect the committed changes. Closed by commit rGf04ccadf3544: [Flang] Add support for fsave-optimization-record (authored by Victor Kingi , committed by kiranchandramohan). Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D155452: [Flang] Add support for fsave-optimization-record

kiranchandramohan added a comment. Please check the failing test. Comment at: flang/test/Driver/fsave-optimization-record.f90:14 +! RUN: rm -f %t.opt.yaml +! RUN: %flang -fsave-optimization-record -S -o %t.o %s +! RUN: cat %t.opt.yaml | FileCheck %s

[PATCH] D155279: [Flang] Include logical default with default-integer-8

This revision was automatically updated to reflect the committed changes. Closed by commit rGfc43c4f0181b: [Flang] Include logical default with default-integer-8 (authored by kiranchandramohan). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: http

[PATCH] D155452: [Flang] Add support for fsave-optimization-record

kiranchandramohan added a comment. Couple of quick comments. You probably need another frontend-forwarding test to check `fsave-optimization-record=`. Comment at: clang/lib/Driver/ToolChains/Flang.cpp:392-395 +static void renderRemarksOptions(const ArgList &Args, ArgStringLis

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

kiranchandramohan added inline comments. Comment at: flang/lib/Lower/Bridge.cpp:271 +bridge{bridge}, foldingContext{bridge.createFoldingContext()}, +ompRequiresFlags{mlir::omp::ClauseRequires::none} {} virtual ~FirConverter() = default; Is this

[PATCH] D150354: [OpenMP][MLIR][Flang][bbc][Driver] Add fopenmp-version and generate corresponding MLIR attribute

kiranchandramohan accepted this revision. kiranchandramohan added a comment. This revision is now accepted and ready to land. LGTM. Target-related constructs came in as part of OpenMP 4.0. Would you want to have a different default for the device version? CHANGES SINCE LAST ACTION https://re

[PATCH] D150354: [OpenMP][MLIR][Flang][bbc][Driver] Add fopenmp-version and generate corresponding MLIR attribute

kiranchandramohan added a comment. In D150354#4342146 , @domada wrote: > In D150354#4337148 , @awarzynski > wrote: > >> All in all LGTM, but I'm not sure whether Flang should be defaulting to >> OpenMP 5.0. AFAI

[PATCH] D148370: [Clang][Flang][OpenMP] Add loadOffloadInfoMetadata and createOffloadEntriesAndInfoMetadata into OMPIRBuilder's finalize and initialize

kiranchandramohan accepted this revision. kiranchandramohan added a comment. This revision is now accepted and ready to land. LG. Please wait a day before submitting. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:455-468 + auto Buf = MemoryBuffer::getFile(HostFilePath)

[PATCH] D146557: [MLIR][OpenMP] Refactoring how map clause is processed

kiranchandramohan added a comment. In D146557#4301955 , @TIFitis wrote: > In D146557#4295550 , > @kiranchandramohan wrote: > >> In D146557#4292223 , @TIFitis >> wrote: >

[PATCH] D146557: [MLIR][OpenMP] Refactoring how map clause is processed

kiranchandramohan added a comment. In D146557#4292223 , @TIFitis wrote: > Cleaned up how IsBegin argument is passed for createTargetData call. Please submit this directly as an NFC patch. We should always work towards simple patches that are easy to rev

[PATCH] D146557: [MLIR][OpenMP] Refactoring how map clause is processed

kiranchandramohan added a comment. Would it be possible to break this into a few patches like suggested below or similar? 1. Changed to using inlineConvertOmpRegions to generate target data associated region code inline in same block if possible. 2. Changed to calculating map_operand size from

[PATCH] D146557: [MLIR][OpenMP] Refactoring how map clause is processed

kiranchandramohan added inline comments. Comment at: mlir/test/Target/LLVMIR/omptarget-llvm.mlir:4-5 -llvm.func @_QPopenmp_target_data() { - %0 = llvm.mlir.constant(1 : i64) : i64 - %1 = llvm.alloca %0 x i32 {bindc_name = "i", in_type = i32, operand_segment_sizes = array, uniq

[PATCH] D146557: [MLIR][OpenMP] Refactoring how map clause is processed

kiranchandramohan added a comment. Please update the summary to convey all the changes introduced in the patch. From the tests, it looks like there is a substantial change in the IR. I was hoping this to be an NFC change. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBui

[PATCH] D147324: [OpenMP][MLIR][Flang][bbc][Driver] Add OpenMP RTL Flags to Flang and generate omp.FlagsAttr from them

kiranchandramohan accepted this revision. kiranchandramohan added a comment. This revision is now accepted and ready to land. LG. This portion of the patch was accepted in https://reviews.llvm.org/D145264. See inline comment about location of LLVM Dialect tests. You may either remove the LLVM di

[PATCH] D145264: [OpenMP][MLIR][Flang][Driver][bbc] Lower and apply Module FlagsAttr

kiranchandramohan requested changes to this revision. kiranchandramohan added a comment. This revision now requires changes to proceed. Please split this patch into three: 1. Code changes and testing for the driver and the FIR+OpenMP dialect generated. 2. Code changes and test for FIR+OpenMP to L

[PATCH] D142347: [NFC][Clang] Move DebugOptions to llvm/Frontend for reuse in Flang

kiranchandramohan accepted this revision. kiranchandramohan added a comment. This revision is now accepted and ready to land. LGTM. This refactoring helps share code between Clang and Flang. Please wait a couple of days before submitting. Additional notes in support of approval: -> The llvm comm

[PATCH] D146814: [Flang] Add debug flag to enable current debug information pass

kiranchandramohan added inline comments. Comment at: flang/include/flang/Frontend/CodeGenOptions.h:18 +#include "clang/Basic/DebugInfoOptions.h" #include "llvm/Support/CodeGen.h" The following patch might be useful to avoid adding the clang header. Could you

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

kiranchandramohan accepted this revision. kiranchandramohan added a comment. This revision is now accepted and ready to land. Herald added a subscriber: jplehr. LGTM. Please wait a day before submission. Move the flang portion to a separate commit. Comment at: flang/lib/Lower/

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

kiranchandramohan added inline comments. Comment at: mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1357 +/// Process MapOperands for Target Data directives. +static LogicalResult processMapOperand( +llvm::IRBuilderBase &builder, LLVM::ModuleTranslation

[PATCH] D145883: [Flang][RISCV] Emit target features for RISC-V

kiranchandramohan accepted this revision. kiranchandramohan added a comment. LG. Please wait for and OK from @jrtc27. Comment at: clang/lib/Driver/ToolChains/Flang.cpp:112-114 + case llvm::Triple::riscv64: +getTargetFeatures(D, Triple, Args, CmdArgs, /*ForAs*/ false); +

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

kiranchandramohan added inline comments. Herald added a subscriber: sunshaoce. Comment at: mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1357 +/// Process MapOperands for Target Data directives. +static LogicalResult processMapOperand( +llvm::IRBuilderB

[PATCH] D145883: [Flang][RISCV] Emit target features for RISC-V

kiranchandramohan added inline comments. Comment at: clang/lib/Driver/ToolChains/Flang.cpp:112-114 + case llvm::Triple::riscv64: +getTargetFeatures(D, Triple, Args, CmdArgs, /*ForAs*/ false); +break; jrtc27 wrote: > mnadeem wrote: > > identical code, cou

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

kiranchandramohan added inline comments. Comment at: mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1357 +/// Process MapOperands for Target Data directives. +static LogicalResult processMapOperand( +llvm::IRBuilderBase &builder, LLVM::ModuleTranslation

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

kiranchandramohan requested changes to this revision. kiranchandramohan added inline comments. This revision now requires changes to proceed. Comment at: mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1357 +/// Process MapOperands for Target Data directives.

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

kiranchandramohan accepted this revision. kiranchandramohan added a comment. LG. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144864/new/ https://reviews.llvm.org/D144864 ___ cfe-commits mailing list cf

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

kiranchandramohan added a comment. In D144864#4175332 , @agozillon wrote: > In D144864#4175257 , > @kiranchandramohan wrote: > >> LG. See one minor comment in the tests. >> >> I would prefer having an Interface f

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

kiranchandramohan accepted this revision. kiranchandramohan added a comment. LG. See one minor comment in the tests. I would prefer having an Interface for Target Modules if that could be made to work. I guess this can be taken up separately after https://reviews.llvm.org/D144883. ==

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

kiranchandramohan added a comment. Thanks for the update and the replies. See comments inline. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1561 + /// should be placed. + /// \param HasRegion True if the op has a region associated with it, false + /// otherwi

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

kiranchandramohan requested changes to this revision. kiranchandramohan added a comment. This revision now requires changes to proceed. I have some more comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1556 + /// Generator for '#omp target data' + ///

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

kiranchandramohan requested changes to this revision. kiranchandramohan added a comment. This revision now requires changes to proceed. Thanks for making the change. I am still going through the patch, I have a few comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBu

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

kiranchandramohan added a comment. Could you update the discourse thread (https://discourse.llvm.org/t/rfc-omp-module-and-omp-function-vs-dialect-attributes-to-encode-openmp-properties/67998) with the chosen approach and reasons before proceeding? Repository: rG LLVM Github Monorepo CHANGES

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

kiranchandramohan added a comment. In D142914#4145224 , @TIFitis wrote: > In D142914#4144475 , > @kiranchandramohan wrote: > >> Please add tests for the MLIR portion. > > Can you please tell me where to add these

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

kiranchandramohan requested changes to this revision. kiranchandramohan added a comment. This revision now requires changes to proceed. Please add tests for the MLIR portion. Could you also post the full LLVM IR for a construct with the map clause? Comment at: llvm/lib/Fronten

[PATCH] D143704: [Flang] Part one of Feature List action

kiranchandramohan added a comment. FWIW, We have a plugin (https://github.com/llvm/llvm-project/tree/main/flang/examples/FlangOmpReport) that goes over the parse-tree and reports all the OpenMP constructs and clauses seen. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

kiranchandramohan accepted this revision. kiranchandramohan added a comment. LG. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141910/new/ https://reviews.llvm.org/D141910 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.

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

kiranchandramohan added a comment. Please add more description to the summary regarding the scope of this patch. If it is only able to lower specific llvm-dialect types, please call out that. Please also explain the split in work between mlir::Translation and OpenMPIRbuilder. You can also consi

[PATCH] D140415: [flang] stack arrays pass

kiranchandramohan accepted this revision. kiranchandramohan added a comment. This revision is now accepted and ready to land. LGTM. Please wait till end of day Monday before you submit to provide other reviewers with a chance to provide further comments or request changes. You can consider inlin

[PATCH] D140415: [flang] stack arrays pass

kiranchandramohan added a comment. Looks OK. I have a few questions and some minor comments inline. It might be good to pull in a bit of info from the RFC into the Summary, particularly saying why a dataflow analysis is necessary, what operations are involved in the analysis etc. Could we have

[PATCH] D140415: [flang] stack arrays pass

kiranchandramohan added a comment. Thanks for this patch @tblah. I had a first look. See comments inline. Have not gone through the core part in full yet. Comment at: flang/lib/Optimizer/Transforms/StackArrays.cpp:433 + llvm::DenseSet freedValues; + func->walk([&](mlir::func

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

kiranchandramohan added a subscriber: raghavendhra. kiranchandramohan 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::Constan

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

kiranchandramohan requested changes to this revision. kiranchandramohan added a comment. This revision now requires changes to proceed. Use `omp.canonical_loop` once it is in. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105584/new/ https://review

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

kiranchandramohan added a comment. Herald added a subscriber: thopre. Since the plan is to switch to the Canonical Style operation, may be this can wait till the `omp.canonical_loop` changes are in? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105

[PATCH] D142199: [Docs] Replace recommonmark with myst-parser

kiranchandramohan added a comment. The flang website (flang.llvm.org) or flang.llvm.org/docs are built from these. Would you know whether there are changes required are there? https://github.com/llvm/llvm-zorg/blob/main/zorg/buildbot/builders/SphinxDocsBuilder.py Repository: rG LLVM Github M

[PATCH] D142347: [NFC][Clang] Move DebugOptions to llvm/Frontend for reuse in Flang

kiranchandramohan created this revision. Herald added a subscriber: sunshaoce. Herald added a project: All. kiranchandramohan requested review of this revision. Herald added subscribers: llvm-commits, cfe-commits, MaskRay. Herald added projects: clang, LLVM. Repository: rG LLVM Github Monorepo

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

kiranchandramohan accepted this revision. kiranchandramohan added a comment. This revision is now accepted and ready to land. LGTM. Thanks for making the changes and for your patience. Please wait a couple of days to give other reviewers a chance to have a look before submitting. Could you updat

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

kiranchandramohan added inline comments. Comment at: mlir/lib/Dialect/OpenMP/CMakeLists.txt:15 MLIRLLVMDialect + MLIRArithDialect ) TIFitis wrote: > kiranchandramohan wrote: > > TIFitis wrote: > > > kiranchandramohan wrote: > > > > Why is this needed here?

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

kiranchandramohan added inline comments. Comment at: mlir/test/Dialect/OpenMP/ops.mlir:124 +// CHECK-LABEL: omp_DistributeOp +func.func @omp_DistributeOp(%lb : index, %ub : index, %step : index, %data_var : memref, %chunk_var : i32) -> () { abidmalikwaterloo w

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

kiranchandramohan requested changes to this revision. kiranchandramohan added a comment. This revision now requires changes to proceed. Can you add the other authors as Co-authors in the patch Summary? In D131915#4001620 , @TIFitis wrote: >> Can you add

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

kiranchandramohan requested changes to this revision. kiranchandramohan added a comment. This revision now requires changes to proceed. Thanks for the changes and your work here. In D131915#3994160 , @TIFitis wrote: > Added custom printer & parser for ma

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

kiranchandramohan requested changes to this revision. kiranchandramohan added a comment. This revision now requires changes to proceed. @abidmalikwaterloo It seems you missed a few of the previous comments. Please fix these so that we can approve. Comment at: mlir/include/mlir

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

kiranchandramohan added a comment. In D105584#3954144 , @abidmalikwaterloo wrote: > In D105584#3917238 , > @kiranchandramohan wrote: > >> Patch probably needs a rebase. A few more minor things to fix. Looks most

[PATCH] D137995: [Flang][Driver] Handle target CPU and features

kiranchandramohan added a comment. Thanks @mnadeem for this patch. A few minor comments first. Try to replace auto in all places except where the type is on the RHS. We might need `-fc1` tests as well. Comment at: clang/lib/Driver/ToolChains/Flang.cpp:85 +

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

kiranchandramohan requested changes to this revision. kiranchandramohan added a comment. This revision now requires changes to proceed. Patch probably needs a rebase. A few more minor things to fix. Looks mostly ready. Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:

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

kiranchandramohan added a comment. In D137329#3909943 , @awarzynski wrote: > In D137329#3909082 , @clementval > wrote: > >> Wouldn't it be good to have a RFC for all these options and what they will >> do in Fl

[PATCH] D136080: [flang] Add -ffp-contract option processing

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa784de783af5: [flang] Add -ffp-contract option processing (authored by tblah, committed by kiranchandramohan). Repository: rG LLVM Github Monorepo

[PATCH] D131808: [clang,flang] Add help text for -fsyntax-only

kiranchandramohan added a comment. In D131808#3722809 , @alexiprof wrote: > Could someone land this patch please? > Alexander Malkov Was landed with the email id contained in the patch (Author: Alexander Malkov ). Hope that is OK. May be you can update

[PATCH] D131808: [clang,flang] Add help text for -fsyntax-only

This revision was automatically updated to reflect the committed changes. Closed by commit rGcd86a03246a2: [clang,flang] Add help text for -fsyntax-only (authored by alexiprof, committed by kiranchandramohan). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D131808: [clang,flang] Add help text for -fsyntax-only

kiranchandramohan added a comment. I can land this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131808/new/ https://reviews.llvm.org/D131808 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[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

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] D110114: [OMPIRBuilder] Generate aggregate argument for parallel region outlined functions

kiranchandramohan added subscribers: Meinersbur, kiranchandramohan. kiranchandramohan added a comment. Herald added a project: All. If I understand this correctly, filling the aggregate struct is only happening in the parallel case but not for the serialized parallel version. See example below,

[PATCH] D128333: [clang][flang] Disable defaulting to `-fpie` for LLVM Flang

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

[PATCH] D128333: [clang][flang] Disable defaulting to `-fpie` for LLVM Flang

kiranchandramohan added a comment. Is the longer-term plan to support this in Flang as well? Does this affect building object files too or is it just the executable? How would this affect mixed C++/Fortran programs if the Clang and Flang settings are different? Repository: rG LLVM Github Mo

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

kiranchandramohan added a comment. In D125788#3535199 , @sscalpone wrote: > My proposal is: > > If the compiler compiles it, it ought to run. > If the compiler can't compile it, it ought to clearly say why. > > 1. All tests of legal Fortran that compile &

[PATCH] D126291: [flang][Driver] Update link job on windows

kiranchandramohan added a comment. @rovka in case you missed this, the test is failing in windows. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126291/new/ https://reviews.llvm.org/D126291 ___ cfe-commi

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

kiranchandramohan added a comment. In D125788#3523408 , @shraiysh wrote: >> With this change the flang driver will not generate an executable unless it >> is built with an option. > > Okay, thank you for the clarification. Is this a cmake option and? Are

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

kiranchandramohan added a comment. In D125788#3523259 , @shraiysh wrote: >> clarify that execution is still restricted to developers via a flag. > > Just confirming what this means: After this patch, would `flang sample.f90` > generate an executable? If

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

kiranchandramohan accepted this revision. kiranchandramohan added a comment. This revision is now accepted and ready to land. LGTM. In the call on Monday, it was mentioned that the original users of the flang script have moved on to using a custom driver. It will be a good idea to, -> wait for t

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

kiranchandramohan accepted this revision. kiranchandramohan added a comment. This revision is now accepted and ready to land. LGTM. You may wait for a day in case there are comments from other reviewers. I think the CI is complaining about some clang-format issue in clang/test/OpenMP/critical_c

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

kiranchandramohan added a comment. Thanks @Meinersbur for this patch. Apologies for the delay in reviewing many of your patches. The patch looks mostly LGTM. A few nits. The patch probably needs a rebase since there are no dependencies now and a few more construct lowering has landed in MLIR

[PATCH] D119012: [flang][driver] Add support for the `-emit-llvm` option

kiranchandramohan added a comment. The tests added are failing in the windows CI. Comment at: flang/lib/Frontend/FrontendActions.cpp:421 + // Set-up the MLIR pass manager + fir::setTargetTriple(*mlirModule_, "native"); + auto &defKinds = ci.invocation().semanticsContext().de

[PATCH] D118985: [flang][driver] Add support for `-emit-mlir`

kiranchandramohan accepted this revision. kiranchandramohan added a comment. LGTM. Comment at: flang/lib/Frontend/FrontendActions.cpp:419 + + // ... otherwise, print to a file. + auto os{ci.CreateDefaultOutputFile( awarzynski wrote: > kiranchandramohan wrote:

[PATCH] D118985: [flang][driver] Add support for `-emit-mlir`

kiranchandramohan added a comment. Few Nits/questions. Comment at: flang/lib/Frontend/FrontendActions.cpp:69 + + // Create a LoweringBridge + auto &defKinds = ci.invocation().semanticsContext().defaultKinds(); Nit: Can we remove the three autos below? =

[PATCH] D111992: [MLIR][OpenMP] Added omp.atomic.read and omp.atomic.write

kiranchandramohan accepted this revision. kiranchandramohan added a comment. This revision is now accepted and ready to land. LGTM. My preference is the following one. If you agree then please make the change, otherwise, you can keep it as is. Also, wait a couple of days to see whether others h

[PATCH] D111992: [MLIR][OpenMP] Added omp.atomic.read and omp.atomic.write

kiranchandramohan added a comment. The patch looks OK. I just wanted to discuss the syntax also. Would any of the following be better? %8 = omp.atomic.read %addr : memref -> i32 hint(speculative, contended) memory_order(seq_cst) %8 = omp.atomic.read %addr hint(speculative, contended) memo

[PATCH] D107430: [OMPIRBuilder] Add ordered directive to OMPIRBuilder

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

[PATCH] D107430: [OMPIRBuilder] Add ordered directive to OMPIRBuilder

kiranchandramohan added a comment. Thanks @peixin. I am going through the patch today and will accept or provide some comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107430/new/ https://reviews.llvm.org/D107430 ___ cfe-commits mailin

[PATCH] D107764: [OpenMP][OpenMPIRBuilder] Implement loop unrolling.

kiranchandramohan accepted this revision. kiranchandramohan added a comment. LGTM. Thanks for the responses. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107764/new/ https://reviews.llvm.org/D107764 ___

[PATCH] D107764: [OpenMP][OpenMPIRBuilder] Implement loop unrolling.

kiranchandramohan added a comment. I have a few minor questions. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2055 +/// Attach loop metadata \p Properties to the loop described by \p Loop. If the +/// loop already has metadata, the loop properties are appended. +static

[PATCH] D107430: [OMPIRBuilder] Add ordered without depend and simd clause to OMPBuilder

kiranchandramohan added a comment. Thanks @peixin for the patch. Could you remove the OpenMP 1.0 target from the description? That is a target for the Flang team and since this patch touches clang as well, it might be confusing. I believe this is modelling the ordered construct with a region. C

[PATCH] D93373: [Flang][Openmp] Upgrade TASKGROUP construct to 5.0.

kiranchandramohan accepted this revision. kiranchandramohan added a comment. This revision is now accepted and ready to land. LGTM. Please update the commit message with information about changes in Clang tests as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

  1   2   >