[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. 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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. 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 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. In D146557#4301955 , @TIFitis wrote: > In D146557#4295550 , > @kiranchandramohan wrote: > >> In D146557#4292223 , @TIFitis >> wrote: >

[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] 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] 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] D98191: [flang][driver] Add support for `-fdebug-dump-symbols-sources`

kiranchandramohan added a comment. In D98191#2611694 , @tskeith wrote: > `-fget-symbols-sources` is not a debug option, it's intended for integrating > with IDEs like vscode. So I think the original name is better. Unlike the > "dump" options it actually

[PATCH] D98191: [flang][driver] Add support for `-fdebug-dump-symbols-sources`

kiranchandramohan accepted this revision. kiranchandramohan added inline comments. This revision is now accepted and ready to land. Comment at: flang/include/flang/Frontend/FrontendOptions.h:60 + + /// Parse, run semantics and the dump symbol sources map + GetSymbolsSources ---

[PATCH] D99292: [flang][driver] Add support for `-cpp/-nocpp`

kiranchandramohan added a comment. minor comments. Comment at: flang/lib/Frontend/FrontendAction.cpp:73 + // Include standard macro predefinitions (use the file + // extension if the user didn't express any prefernce) + if ((invoc.preprocessorOpts().predefs_ == StdMacroPred

[PATCH] D99292: [flang][driver] Add support for `-cpp/-nocpp`

kiranchandramohan accepted this revision. kiranchandramohan added a comment. This revision is now accepted and ready to land. LGTM. Comment at: flang/include/flang/Frontend/CompilerInvocation.h:142 + /// + /// \param [in] ppOpts The preprocessor options + void collectMacroDe

[PATCH] D99645: [flang][driver] Add debug options not requiring semantic checks

kiranchandramohan added inline comments. Comment at: clang/include/clang/Driver/Options.td:4378 HelpText<"Run the InputOuputTest action. Use for development and testing only.">; +def fdebug_unparse_no_sema : Flag<["-"], "fdebug-unparse-no-sema">, Group, + HelpText<"Unparse

[PATCH] D99645: [flang][driver] Add debug options not requiring semantic checks

kiranchandramohan accepted this revision. kiranchandramohan added a comment. LGTM. Might be good to have a test for fdebug-dump-parse-tree-no-sema. Comment at: flang/include/flang/Frontend/FrontendOptions.h:54 + /// Parse, run semantics and then output the parse tree, skip

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

<    1   2