[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

kiranchandramohan added a comment. Some minor comments about Filetypes and file extensions. Can be ignored or considered for a separate commit. Comment at: clang/include/clang/Driver/Driver.h:69 +CLMode, +FlangMode, } Mode; Is the comma by choice?

[PATCH] D79677: [Clang][OpenMP][OMPBuilder] (1/4) Privatize `parallel` for `OMPBuilder`

kiranchandramohan added a comment. Thanks @fghanim for this patch. I will get to this on Friday if you can add me as a reviewer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79677/new/ https://reviews.llvm.org/D79677 __

[PATCH] D79677: [Clang][OpenMP][OMPBuilder] (1/4) Privatize `parallel` for `OMPBuilder`

kiranchandramohan added a comment. Is the ordering of code generation for clauses important? copyin -> firstprivate -> barrier -> private Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:1673 // // TODO: This defaults to shared right now. auto PrivCB = [](InsertPoi

[PATCH] D85619: [clang][OpenMP][OMPBuilder] Use OMPBuilder to CG `omp single`

kiranchandramohan added a comment. Before I start, Why is this change all new code and no modification or deletion of existing Clang functionality for omp single/copyprivate? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85619/new/ https://reviews

[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] 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

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] 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://

[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] 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] 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] 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] 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] 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] 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 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] D87774: [flang] Introduce DiagnosticConsumer classes in libflangFrontend

kiranchandramohan added a comment. I had a quick look through this patch. Have a few comments (mostly nits and questions) inline. Comment at: flang/include/flang/Frontend/CompilerInvocation.h:18 + +/// Fill out Opts based on the options given in Args. +/// Nit

[PATCH] D87774: [flang] Introduce DiagnosticConsumer classes in libflangFrontend

kiranchandramohan added inline comments. Comment at: flang/lib/Frontend/TextDiagnosticPrinter.cpp:24 + +TextDiagnosticPrinter::TextDiagnosticPrinter( +raw_ostream &os, clang::DiagnosticOptions *diags) awarzynski wrote: > sameeranjoshi wrote: > > A silly quest

[PATCH] D87774: [flang] Introduce DiagnosticConsumer classes in libflangFrontend

kiranchandramohan added a subscriber: rovka. kiranchandramohan added a comment. In D87774#2298515 , @awarzynski wrote: > In D87774#2293283 , @sameeranjoshi > wrote: > >> Do you know if there are any bots configured

[PATCH] D85619: [clang][OpenMP][OMPBuilder] Use OMPBuilder to CG `omp single`

kiranchandramohan added a comment. What is the plan for this patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85619/new/ https://reviews.llvm.org/D85619 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D79677: [Clang][OpenMP][OMPBuilder] (1/4) Privatize `parallel` for `OMPBuilder`

kiranchandramohan added a comment. What is the plan for this patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79677/new/ https://reviews.llvm.org/D79677 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D92854: [flang][driver] Add support for `-fsyntax-only`

kiranchandramohan added a comment. A few comments. Comment at: clang/lib/Driver/ToolChains/Flang.cpp:43-44 } else if (isa(JA) || isa(JA)) { -CmdArgs.push_back("-triple"); -CmdArgs.push_back(Args.MakeArgString(TripleStr)); if (JA.getType() == types::TY_Nothing) {

[PATCH] D90009: [X86] VEX/EVEX prefix doesn't work for inline assembly.

kiranchandramohan added a comment. This change probably requires the X86 target. // REQUIRES: x86-registered-target Builds which target AArch64 only have been failing due to this change. http://lab.llvm.org:8011/#/builders/32/builds/291 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D90009: [X86] VEX/EVEX prefix doesn't work for inline assembly.

kiranchandramohan added a comment. I have added a fix to run the test only when the X86 target is available. Please feel free to change if it is not the correct fix. https://github.com/llvm/llvm-project/commit/c551ba0e90bd2b49ef501d591f8362ba44e5484d Repository: rG LLVM Github Monorepo CHANG

[PATCH] D88609: Use uint64_t for branch weights instead of uint32_t

kiranchandramohan added a comment. Updated mlir test also to include this change. https://reviews.llvm.org/rG0fc1aa22ee6ac337a5d51fa5666c9cd61da61b07 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88609/new/ https://reviews.llvm.org/D88609

[PATCH] D88609: Use uint64_t for branch weights instead of uint32_t

kiranchandramohan added a subscriber: thakis. kiranchandramohan added a comment. @thakis Can you revert the testcase fix in MLIR also which was necessary after this patch? https://reviews.llvm.org/rG0fc1aa22ee6ac337a5d51fa5666c9cd61da61b07 Repository: rG LLVM Github Monorepo CHANGES SINCE LA

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

kiranchandramohan added a comment. It could be because you are not the author. Can you commandeer the patch (option available in the Add Action menu below)? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93373/new/ https://reviews.llvm.org/D93373

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

kiranchandramohan added a comment. Can you rebase and update the patch? It seems to have built for the latest revision (Harbormaster completed remote builds in B110336 : Diff 353548). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[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] D91556: [OpenMPIRBuilder} Add capturing of parameters to pass to omp::parallel

kiranchandramohan added a comment. Herald added subscribers: dcaballe, cota. Ping @llitchev. Would you have time to take this forward? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91556/new/ https://reviews.llvm.org/D91556 ___

[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. 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] 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. 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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 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] 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] 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] 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] 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] 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] 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] 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#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] 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. 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] 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] 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] 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] 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] 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] 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 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] 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] 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] 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] 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] 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] 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] 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.

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] 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] 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. 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] 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] 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] 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] 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] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

kiranchandramohan added a comment. Will tests come in a later patch? Comment at: clang/include/clang/AST/OpenMPClause.h:6700 /// independent of clang. Thus, expressions and conditions are evaluated in - /// this method. + /// this method. If \p DeviceSetOnly is set only th

[PATCH] D69922: [OpenMP] Use the OpenMP-IR-Builder

kiranchandramohan added a comment. Thanks @jdoerfert for working on this. Sorry for being late to the party. I am trying to implement one trivial directive (Flush) and one slightly more involved (not decided). I applied D69853 , D69785

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

kiranchandramohan added a comment. Had a quick look today. Will spend some more time tomorrow. Please see a few comments inline. Comment at: clang/include/clang/AST/OpenMPClause.h:6429 +/// and an ordered collection of properties. +struct OpenMPTraitInfo { + struct OpenMPTrait

[PATCH] D70712: Adding Flush support in OpenMPIRBuilder

This revision was automatically updated to reflect the committed changes. Closed by commit rGa969e051a5d9: [OpenMP] Add Flush directive to OpenMPIRBuilder (authored by kiranchandramohan). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https://rev

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

kiranchandramohan added inline comments. Comment at: clang/include/clang/Serialization/ASTRecordReader.h:272 + + template <> OpenMPTraitInfo *readUserType() { return readOpenMPTraitInfo(); } + jdoerfert wrote: > kiranchandramohan wrote: > > Compiler throws up th

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

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

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

kiranchandramohan added a comment. Looks OK to me. Couple of comments inline. Comment at: clang/include/clang/Serialization/ASTRecordReader.h:272 + + template <> OpenMPTraitInfo *readUserType() { return readOpenMPTraitInfo(); } + jdoerfert wrote: > jdoerfert

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

kiranchandramohan added inline comments. Comment at: clang/lib/Serialization/ASTWriter.cpp:6575 +template <> void ASTRecordWriter::writeUserType(OMPTraitInfo *TI) { + writeUInt32(TI->Sets.size()); Had to also move this up in the file to avoid an instantiation

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

kiranchandramohan accepted this revision. kiranchandramohan added a comment. This revision is now accepted and ready to land. LGTM. You can wait for a day in case other reviewers have comments. Comment at: clang/lib/Serialization/ASTWriter.cpp:6575 +template <> void ASTRecord

[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] 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] 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] 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 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] 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 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] 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] 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. 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] 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] 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 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. 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

  1   2   >