[llvm] [flang] [clang] [NFC][AMDGPU] Move address space enum to LLVM directory (PR #73944)
https://github.com/skatrak approved this pull request. LGTM, thanks! Please wait for one more approval before merging. https://github.com/llvm/llvm-project/pull/73944 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [Flang] Add code-object-version option (PR #72638)
https://github.com/skatrak commented: Thanks for this work, Dominik. It LGTM, but please wait for another review before merging. I have a couple of suggestions for small improvements, but let me know if they would take significant effort to address, as I'm not completely familiar with that part of the project. https://github.com/llvm/llvm-project/pull/72638 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang] Add code-object-version option (PR #72638)
https://github.com/skatrak edited https://github.com/llvm/llvm-project/pull/72638 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [Flang] Add code-object-version option (PR #72638)
@@ -264,6 +263,37 @@ static void addDepdendentLibs(mlir::ModuleOp &mlirModule, } } +// Add to MLIR code target specific items which are dependent on target +// configuration specified by the user +static void addTargetSpecificMLIRItems(mlir::ModuleOp &mlirModule, + CompilerInstance &ci) { + const TargetOptions &targetOpts = ci.getInvocation().getTargetOpts(); + const llvm::Triple triple(targetOpts.triple); + if (triple.isAMDGPU()) { +unsigned oclcABIVERsion; +const unsigned defaultOclcABIVERsion = 400; +mlir::OpBuilder builder(mlirModule.getContext()); +const CodeGenOptions &codeGenOpts = ci.getInvocation().getCodeGenOpts(); +if (codeGenOpts.CodeObjectVersion == +CodeGenOptions::CodeObjectVersionKind::COV_None) + oclcABIVERsion = defaultOclcABIVERsion; +else + oclcABIVERsion = static_cast(codeGenOpts.CodeObjectVersion); + +auto int32Type = builder.getI32Type(); +auto covInfo = builder.create( +mlirModule.getLoc(), int32Type, true, mlir::LLVM::Linkage::WeakODR, +"__oclc_ABI_version", +builder.getIntegerAttr(int32Type, oclcABIVERsion)); +covInfo.setUnnamedAddr(mlir::LLVM::UnnamedAddr::Local); +covInfo.setAddrSpace(4); skatrak wrote: Is this address space value defined in any enumeration that could be used here, rather than passing just a number? Of if they are documented somewhere, at least refer to it in a comment. https://github.com/llvm/llvm-project/pull/72638 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [Flang] Add code-object-version option (PR #72638)
@@ -264,6 +263,37 @@ static void addDepdendentLibs(mlir::ModuleOp &mlirModule, } } +// Add to MLIR code target specific items which are dependent on target +// configuration specified by the user +static void addTargetSpecificMLIRItems(mlir::ModuleOp &mlirModule, + CompilerInstance &ci) { + const TargetOptions &targetOpts = ci.getInvocation().getTargetOpts(); + const llvm::Triple triple(targetOpts.triple); + if (triple.isAMDGPU()) { +unsigned oclcABIVERsion; +const unsigned defaultOclcABIVERsion = 400; skatrak wrote: Would it be possible/advisable to have this constant somewhere shared between clang and flang, so they are guaranteed to default to the same ABI version? https://github.com/llvm/llvm-project/pull/72638 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [llvm] [clang] [flang][OpenMP] Add semantic check for declare target (PR #72770)
@@ -1851,6 +1853,18 @@ bool ClauseProcessor::processTo( }); } +bool ClauseProcessor::processEnter( +llvm::SmallVectorImpl &result) const { + return findRepeatableClause( + [&](const ClauseTy::Enter *enterClause, + const Fortran::parser::CharBlock &) { +// Case: declare target to(func, var1, var2)... skatrak wrote: Nit: Update comment https://github.com/llvm/llvm-project/pull/72770 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [flang] [flang][OpenMP] Add semantic check for declare target (PR #72770)
https://github.com/skatrak edited https://github.com/llvm/llvm-project/pull/72770 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [flang] [flang][OpenMP] Add semantic check for declare target (PR #72770)
@@ -590,6 +590,8 @@ class ClauseProcessor { bool processSectionsReduction(mlir::Location currentLocation) const; bool processTo(llvm::SmallVectorImpl &result) const; bool + processEnter(llvm::SmallVectorImpl &result) const; skatrak wrote: Nit: It would be good to keep the alphabetic order of declarations here and definitions below, so I'd add this between `processDeviceType` and `processFinal`. https://github.com/llvm/llvm-project/pull/72770 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [flang][OpenMP] Add semantic check for declare target (PR #72770)
https://github.com/skatrak commented: Thank you Shraiysh, I just have a couple of nits. https://github.com/llvm/llvm-project/pull/72770 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [flang] [NFC][AMDGPU] Move address space enum to LLVM directory (PR #73944)
https://github.com/skatrak commented: Thanks for this work. I only have very little feedback, but it LGTM. I think the enum looks a bit out of place in a file called "TargetParser.h", but I'm not very familiar with this part of the project, so I'll let the experts say something if there is a more suitable location for it. https://github.com/llvm/llvm-project/pull/73944 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [flang] [NFC][AMDGPU] Move address space enum to LLVM directory (PR #73944)
https://github.com/skatrak edited https://github.com/llvm/llvm-project/pull/73944 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [flang] [clang] [NFC][AMDGPU] Move address space enum to LLVM directory (PR #73944)
@@ -405,9 +398,9 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUTargetInfo final : public TargetInfo { getDWARFAddressSpace(unsigned AddressSpace) const override { const unsigned DWARF_Private = 1; const unsigned DWARF_Local = 2; -if (AddressSpace == Private) { +if (AddressSpace == llvm::AMDGPU::Private) { skatrak wrote: Nit: switch? https://github.com/llvm/llvm-project/pull/73944 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [flang] [NFC][AMDGPU] Move address space enum to LLVM directory (PR #73944)
@@ -31,6 +31,15 @@ class Triple; // back-end to TableGen to create these clean tables. namespace AMDGPU { +/// Address space values for AMD GPUs +enum AddrSpace { + Generic = 0, skatrak wrote: I think it would be good to prefix these names with something so that uses are self-documenting. For example, something like AS_{GENERIC, GLOBAL, ...} or ADDRESS_{GENERIC, GLOBAL, ...}, if we use the enums below as reference. https://github.com/llvm/llvm-project/pull/73944 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Frontend] Add leaf constructs and association to OpenMP/ACC directives (PR #83625)
@@ -665,60 +619,44 @@ bool clang::isOpenMPTargetDataManagementDirective(OpenMPDirectiveKind DKind) { } bool clang::isOpenMPNestingTeamsDirective(OpenMPDirectiveKind DKind) { - return DKind == OMPD_teams || DKind == OMPD_teams_distribute || - DKind == OMPD_teams_distribute_simd || - DKind == OMPD_teams_distribute_parallel_for_simd || - DKind == OMPD_teams_distribute_parallel_for || - DKind == OMPD_teams_loop; + if (DKind == OMPD_teams) +return true; + auto leafs = getLeafConstructs(DKind); skatrak wrote: > Leafs is an accepted form. Could you point to some source where that's described? After a quick search, I find that "leaves" is the widely accepted plural of "leaf", and that "leafs" is only recognized by certain dictionaries. One instance where it's not: https://dictionary.cambridge.org/dictionary/english/leaf. https://github.com/llvm/llvm-project/pull/83625 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[openmp] [clang] [llvm] [OpenMP] Remove `register_requires` global constructor (PR #80460)
https://github.com/skatrak edited https://github.com/llvm/llvm-project/pull/80460 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[openmp] [llvm] [clang] [OpenMP] Remove `register_requires` global constructor (PR #80460)
https://github.com/skatrak commented: I noticed a small breakage of the OpenMP MLIR dialect from one of these changes. It should be trivial to address. https://github.com/llvm/llvm-project/pull/80460 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [openmp] [OpenMP] Remove `register_requires` global constructor (PR #80460)
@@ -6872,35 +6883,6 @@ void OpenMPIRBuilder::loadOffloadInfoMetadata(StringRef HostFilePath) { loadOffloadInfoMetadata(*M.get()); } -Function *OpenMPIRBuilder::createRegisterRequires(StringRef Name) { skatrak wrote: Removing this function breaks the compilation of the OpenMP MLIR dialect, as it's used there (mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp: `convertRequiresAttr()`). My understanding is that creating this function would no longer be necessary, so the solution to that should be to remove `convertRequiresAttr()` and replace the call to it in `OpenMPDialectLLVMIRTranslationInterface::amendOperation()` to `return success()` in the same file. Flang should already pick up your other changes, so REQUIRES information should still work in Fortran after this. https://github.com/llvm/llvm-project/pull/80460 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [openmp] [mlir] [OpenMP] Remove `register_requires` global constructor (PR #80460)
@@ -6872,35 +6883,6 @@ void OpenMPIRBuilder::loadOffloadInfoMetadata(StringRef HostFilePath) { loadOffloadInfoMetadata(*M.get()); } -Function *OpenMPIRBuilder::createRegisterRequires(StringRef Name) { skatrak wrote: Flang already picks up your changes to `createOffloadEntriesAndInfoMetadata()` and it appears to be putting the corresponding flags into a structure, though I'm not sure it's the right one. I ran `flang-new -fc1 -emit-llvm -fopenmp flang/test/Lower/OpenMP/requires.f90 -o -` for a simple test with a single target region and I'm seeing two offloading entries being produced, which I'm not sure whether it's expected behavior. It looks like only one of them has the requires flags and the other one is the one that's properly linked with the kernel. ```llvmir @.omp_offloading.entry_name = internal unnamed_addr constant [43 x i8] c"__omp_offloading_10307_d2a215c__QQmain_l12\00" @.omp_offloading.entry.__omp_offloading_10307_d2a215c__QQmain_l12 = weak constant %struct.__tgt_offload_entry { ptr @.__omp_offloading_10307_d2a215c__QQmain_l12.region_id, ptr @.omp_offloading.entry_name, i64 0, i32 0, i32 0 }, section "omp_offloading_entries", align 1 @.omp_offloading.entry_name.1 = internal unnamed_addr constant [1 x i8] zeroinitializer @.omp_offloading.entry. = weak constant %struct.__tgt_offload_entry { ptr null, ptr @.omp_offloading.entry_name.1, i64 0, i32 22, i32 10 }, section "omp_offloading_entries", align 1 ... %14 = call i32 @__tgt_target_kernel(ptr @1, i64 -1, i32 -1, i32 0, ptr @.__omp_offloading_10307_d2a215c__QQmain_l12.region_id, ptr %kernel_args) ``` https://github.com/llvm/llvm-project/pull/80460 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [openmp] [clang] [mlir] [OpenMP] Remove `register_requires` global constructor (PR #80460)
@@ -6872,35 +6883,6 @@ void OpenMPIRBuilder::loadOffloadInfoMetadata(StringRef HostFilePath) { loadOffloadInfoMetadata(*M.get()); } -Function *OpenMPIRBuilder::createRegisterRequires(StringRef Name) { skatrak wrote: The test itself has "requires unified_shared_memory reverse_offload" (it originally just checks that the flags are passed to MLIR), so that's where the "10" is coming from. The "22" I don't know what it's supposed represent. https://github.com/llvm/llvm-project/pull/80460 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [openmp] [OpenMP] Remove `register_requires` global constructor (PR #80460)
@@ -6872,35 +6883,6 @@ void OpenMPIRBuilder::loadOffloadInfoMetadata(StringRef HostFilePath) { loadOffloadInfoMetadata(*M.get()); } -Function *OpenMPIRBuilder::createRegisterRequires(StringRef Name) { skatrak wrote: > It was a very obvious problem. I mixed up hexadecimal and did `0x16` when I > meant either `0x10` or `16`. Fixed now. Oh, then the "16" bit just happened to be part of the number that was being set. Glad to hear this wasn't some unexpected interaction specific to Flang. https://github.com/llvm/llvm-project/pull/80460 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [openmp] [OpenMP] Remove `register_requires` global constructor (PR #80460)
https://github.com/skatrak edited https://github.com/llvm/llvm-project/pull/80460 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[lldb] [libc] [clang] [llvm] [libcxxabi] [clang-tools-extra] [lld] [flang] [compiler-rt] [libcxx] [Flang][OpenMP] Push genEval calls to individual operations, NFC (PR #77758)
@@ -3358,14 +3396,17 @@ genOMP(Fortran::lower::AbstractConverter &converter, .t); // Currently only private/firstprivate clause is handled, and // all privatization is done within `omp.section` operations. + symTable.pushScope(); skatrak wrote: Thanks for the explanation, that clears it for me https://github.com/llvm/llvm-project/pull/77758 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [libcxx] [compiler-rt] [flang] [libc] [lldb] [clang-tools-extra] [lld] [libcxxabi] [Flang][OpenMP] Push genEval calls to individual operations, NFC (PR #77758)
https://github.com/skatrak approved this pull request. Thanks for addressing my comments, LGTM. https://github.com/llvm/llvm-project/pull/77758 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[lldb] [libc] [clang] [llvm] [libcxxabi] [clang-tools-extra] [lld] [flang] [compiler-rt] [libcxx] [Flang][OpenMP] Push genEval calls to individual operations, NFC (PR #77758)
https://github.com/skatrak edited https://github.com/llvm/llvm-project/pull/77758 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [lld] [compiler-rt] [flang] [clang] [libcxxabi] [lldb] [libcxx] [clang-tools-extra] [libc] [Flang][OpenMP] Push genEval calls to individual operations, NFC (PR #77758)
@@ -110,6 +110,34 @@ static void gatherFuncAndVarSyms( } } +static Fortran::lower::pft::Evaluation * +getCollapsedEval(Fortran::lower::pft::Evaluation &eval, int collapseValue) { skatrak wrote: Nit: Maybe `getCollapsedLoopEval` would be slightly more self-explanatory? https://github.com/llvm/llvm-project/pull/77758 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [flang] [llvm] [clang-tools-extra] [Flang][OpenMP] Push genEval closer to leaf lowering functions (PR #77760)
@@ -2178,7 +2178,7 @@ createAndSetPrivatizedLoopVar(Fortran::lower::AbstractConverter &converter, template static void createBodyOfOp( Op &op, Fortran::lower::AbstractConverter &converter, mlir::Location &loc, -Fortran::lower::pft::Evaluation &eval, +Fortran::lower::pft::Evaluation &eval, bool genNested, skatrak wrote: I guess this is just a matter of taste, so feel free to leave it as is. The reason why I suggested the change is that there are two boolean flags passed in: `genNested` and `outerCombined`. The first one indicates _what_ it will do and the second indicates _when_ the caller should set it to true, so I thought it would be good if both conveyed information in the same way. https://github.com/llvm/llvm-project/pull/77760 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [flang] [llvm] [clang-tools-extra] [Flang][OpenMP] Push genEval closer to leaf lowering functions (PR #77760)
https://github.com/skatrak approved this pull request. LGTM, thank you! https://github.com/llvm/llvm-project/pull/77760 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [OMPIRBuilder] - Handle dependencies in `createTarget` (PR #93977)
https://github.com/skatrak approved this pull request. Thank you Pranav, LGTM. I have just one more comment but no need to wait for another review by me before merging this. Just make sure @Meinersbur is happy with it as well! https://github.com/llvm/llvm-project/pull/93977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [OMPIRBuilder] - Handle dependencies in `createTarget` (PR #93977)
https://github.com/skatrak edited https://github.com/llvm/llvm-project/pull/93977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [OMPIRBuilder] - Handle dependencies in `createTarget` (PR #93977)
@@ -682,6 +682,41 @@ convertOmpTeams(omp::TeamsOp op, llvm::IRBuilderBase &builder, return bodyGenStatus; } +static void +buildDependData(Operation *op, LLVM::ModuleTranslation &moduleTranslation, skatrak wrote: I'd suggest taking `std::optional depends` and `OperandRange dependVars` as arguments instead of `Operation *op` here. This way you don't need to try casting `op` to each supported operation below, but rather make the body of this function what the `processDepends` lambda inside currently has. https://github.com/llvm/llvm-project/pull/93977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [OMPIRBuilder] - Handle dependencies in `createTarget` (PR #93977)
@@ -705,28 +740,7 @@ convertOmpTaskOp(omp::TaskOp taskOp, llvm::IRBuilderBase &builder, }; SmallVector dds; - if (!taskOp.getDependVars().empty() && taskOp.getDepends()) { -for (auto dep : - llvm::zip(taskOp.getDependVars(), taskOp.getDepends()->getValue())) { - llvm::omp::RTLDependenceKindTy type; - switch ( - cast(std::get<1>(dep)).getValue()) { - case mlir::omp::ClauseTaskDepend::taskdependin: -type = llvm::omp::RTLDependenceKindTy::DepIn; -break; - // The OpenMP runtime requires that the codegen for 'depend' clause for - // 'out' dependency kind must be the same as codegen for 'depend' clause - // with 'inout' dependency. - case mlir::omp::ClauseTaskDepend::taskdependout: - case mlir::omp::ClauseTaskDepend::taskdependinout: -type = llvm::omp::RTLDependenceKindTy::DepInOut; -break; - }; - llvm::Value *depVal = moduleTranslation.lookupValue(std::get<0>(dep)); - llvm::OpenMPIRBuilder::DependData dd(type, depVal->getType(), depVal); - dds.emplace_back(dd); -} - } + buildDependData(taskOp.getOperation(), moduleTranslation, dds); skatrak wrote: Suggestion to how this call could look if following my comment above: ```suggestion buildDependData(moduleTranslation, taskOp.getDepends(), taskOp.getDependVars(), dds); ``` https://github.com/llvm/llvm-project/pull/93977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Add -mlink-builtin-bitcode option to fc1 (PR #94763)
@@ -223,9 +223,12 @@ class CodeGenAction : public FrontendAction { std::unique_ptr llvmCtx; std::unique_ptr llvmModule; - /// Embeds offload objects given with specified with -fembed-offload-object + /// Embeds offload objects specified with -fembed-offload-object void embedOffloadObjects(); + /// Links in BC libraries spefified with -fmlink-builtin-bitcode skatrak wrote: ```suggestion /// Links in BC libraries spefified with -mlink-builtin-bitcode ``` https://github.com/llvm/llvm-project/pull/94763 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][OpenMP] Add -fopenmp-force-usm option to flang (PR #94359)
https://github.com/skatrak created https://github.com/llvm/llvm-project/pull/94359 This patch enables the `-fopenmp-force-usm` option to be passed to the flang driver, which forwards it to the compiler frontend. This flag, when set, results in the introduction of the `unified_shared_memory` bit to the `omp.requires` attribute of the top-level module operation. This is later combined with any other target device-related REQUIRES clauses that may have been explicitly set in the compilation unit. >From 369850438197a4176c9fe2689ad9e8032ead5488 Mon Sep 17 00:00:00 2001 From: Sergio Afonso Date: Tue, 4 Jun 2024 15:26:38 +0100 Subject: [PATCH] [Flang][OpenMP] Add -fopenmp-force-usm option to flang This patch enables the `-fopenmp-force-usm` option to be passed to the flang driver, which forwards it to the compiler frontend. This flag, when set, results in the introduction of the `unified_shared_memory` bit to the `omp.requires` attribute of the top-level module operation. This is later combined with any other target device-related REQUIRES clauses that may have been explicitly set in the compilation unit. --- clang/include/clang/Driver/Options.td | 2 +- clang/lib/Driver/ToolChains/Flang.cpp | 2 ++ flang/include/flang/Frontend/LangOptions.def | 2 ++ flang/include/flang/Tools/CrossToolHelpers.h | 20 --- flang/lib/Frontend/CompilerInvocation.cpp | 3 +++ flang/lib/Lower/OpenMP/OpenMP.cpp | 4 +++- flang/test/Driver/omp-driver-offload.f90 | 20 +++ flang/test/Lower/OpenMP/force-usm.f90 | 12 +++ .../test/Lower/OpenMP/requires-force-usm.f90 | 15 ++ flang/tools/bbc/bbc.cpp | 15 +- 10 files changed, 81 insertions(+), 14 deletions(-) create mode 100644 flang/test/Lower/OpenMP/force-usm.f90 create mode 100644 flang/test/Lower/OpenMP/requires-force-usm.f90 diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 57f37c5023110..eefec33e6d333 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3574,7 +3574,7 @@ def fopenmp_offload_mandatory : Flag<["-"], "fopenmp-offload-mandatory">, Group< HelpText<"Do not create a host fallback if offloading to the device fails.">, MarshallingInfoFlag>; def fopenmp_force_usm : Flag<["-"], "fopenmp-force-usm">, Group, - Flags<[NoArgumentUnused]>, Visibility<[ClangOption, CC1Option]>, + Flags<[NoArgumentUnused]>, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>, HelpText<"Force behvaior as if the user specified pragma omp requires unified_shared_memory.">, MarshallingInfoFlag>; def fopenmp_target_jit : Flag<["-"], "fopenmp-target-jit">, Group, diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp index 42ca060186fd8..b7abe7b1c19bc 100644 --- a/clang/lib/Driver/ToolChains/Flang.cpp +++ b/clang/lib/Driver/ToolChains/Flang.cpp @@ -766,6 +766,8 @@ void Flang::ConstructJob(Compilation &C, const JobAction &JA, Args.AddAllArgs(CmdArgs, options::OPT_fopenmp_version_EQ); // FIXME: Clang supports a whole bunch more flags here. + if (Args.hasArg(options::OPT_fopenmp_force_usm)) +CmdArgs.push_back("-fopenmp-force-usm"); break; default: // By default, if Clang doesn't know how to generate useful OpenMP code diff --git a/flang/include/flang/Frontend/LangOptions.def b/flang/include/flang/Frontend/LangOptions.def index 2bf10826120a8..d3e1e972d1519 100644 --- a/flang/include/flang/Frontend/LangOptions.def +++ b/flang/include/flang/Frontend/LangOptions.def @@ -42,6 +42,8 @@ LANGOPT(OpenMPVersion, 32, 0) LANGOPT(OpenMPIsTargetDevice, 1, false) /// Generate OpenMP target code only for GPUs LANGOPT(OpenMPIsGPU, 1, false) +/// Generate OpenMP target code only for GPUs +LANGOPT(OpenMPForceUSM, 1, false) /// Enable debugging in the OpenMP offloading device RTL LANGOPT(OpenMPTargetDebug, 32, 0) /// Assume work-shared loops do not have more iterations than participating diff --git a/flang/include/flang/Tools/CrossToolHelpers.h b/flang/include/flang/Tools/CrossToolHelpers.h index 77b68fc6187fa..26fbe51d329c7 100644 --- a/flang/include/flang/Tools/CrossToolHelpers.h +++ b/flang/include/flang/Tools/CrossToolHelpers.h @@ -130,16 +130,16 @@ struct OffloadModuleOpts { OffloadModuleOpts(uint32_t OpenMPTargetDebug, bool OpenMPTeamSubscription, bool OpenMPThreadSubscription, bool OpenMPNoThreadState, bool OpenMPNoNestedParallelism, bool OpenMPIsTargetDevice, - bool OpenMPIsGPU, uint32_t OpenMPVersion, std::string OMPHostIRFile = {}, - bool NoGPULib = false) + bool OpenMPIsGPU, bool OpenMPForceUSM, uint32_t OpenMPVersion, + std::string OMPHostIRFile = {}, bool NoGPULib = false) : OpenMPTargetDebug(OpenMPTargetDebug), OpenMPTeamSubscription(OpenMPTeamSubscription), OpenMPThreadSubscription(OpenMPThr
[clang] [llvm] [mlir] [OMPIRBuilder] - Handle dependencies in `createTarget` (PR #93977)
https://github.com/skatrak edited https://github.com/llvm/llvm-project/pull/93977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [OMPIRBuilder] - Handle dependencies in `createTarget` (PR #93977)
https://github.com/skatrak commented: Thank you Pranav, I think this looks good. I just have a few minor comments. https://github.com/llvm/llvm-project/pull/93977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [OMPIRBuilder] - Handle dependencies in `createTarget` (PR #93977)
@@ -5252,6 +5660,8 @@ static void emitTargetCall(OpenMPIRBuilder &OMPBuilder, IRBuilderBase &Builder, // emitKernelLaunch auto &&EmitTargetCallFallbackCB = [&](OpenMPIRBuilder::InsertPointTy IP) -> OpenMPIRBuilder::InsertPointTy { +LLVM_DEBUG(dbgs() << "EmitTargetCallFallbackCB::Builder = " << &Builder + << "\n"); skatrak wrote: Nit: Is this addition intentional? https://github.com/llvm/llvm-project/pull/93977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [OMPIRBuilder] - Handle dependencies in `createTarget` (PR #93977)
@@ -5572,6 +5999,8 @@ void OpenMPIRBuilder::emitOffloadingArrays( return; Builder.restoreIP(AllocaIP); + LLVM_DEBUG(dbgs() << "Basicblock before emitOffloadingArrays\n" +<< *(Builder.GetInsertBlock()) << "\n"); skatrak wrote: Nit: Is this addition intentional? https://github.com/llvm/llvm-project/pull/93977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [OMPIRBuilder] - Handle dependencies in `createTarget` (PR #93977)
@@ -5212,6 +5273,78 @@ static Function *createOutlinedFunction( return Func; } +// Create an entry point for a target task with the following. +// It'll have the following signature +// void @.omp_target_task_proxy_func(i32 %thread.id, ptr %task) +// This function is called from emitTargetTask once the +// code to launch the target kernel has been outlined already. +static Function *emitProxyTaskFunction(OpenMPIRBuilder &OMPBuilder, skatrak wrote: I agree with the comment by @ergawy, though I'd rather suggest `emitTargetTaskProxyFunction`, since that's what the emitted function is named. Feel free to ignore if you disagree. https://github.com/llvm/llvm-project/pull/93977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [OMPIRBuilder] - Handle dependencies in `createTarget` (PR #93977)
@@ -0,0 +1,37 @@ +! Offloading test checking the use of the depend clause on +! the target construct +! REQUIRES: flang, amdgcn-amd-amdhsa +! UNSUPPORTED: nvptx64-nvidia-cuda +! UNSUPPORTED: nvptx64-nvidia-cuda-LTO +! UNSUPPORTED: aarch64-unknown-linux-gnu +! UNSUPPORTED: aarch64-unknown-linux-gnu-LTO +! UNSUPPORTED: x86_64-pc-linux-gnu +! UNSUPPORTED: x86_64-pc-linux-gnu-LTO + +! RUN: %libomptarget-compile-fortran-run-and-check-generic +program main + integer :: a = 0 + call foo(5, a) + print*, "=== FORTRAN Test passed! ===" + print*, "foo(5) returned ", a, ", expected 6\n" + ! stop 0 +end program main +subroutine foo(N, r) + integer, intent(in) :: N + integer, intent(out) :: r + integer :: z + + z = 1 + !$omp task depend(out: z) shared(z) + z = N skatrak wrote: Would it make sense to perhaps add a relatively long-running loop before setting the value of `z` here? Just to make sure that the target task below has had to wait for the first to finish (rather than being able to assume it did because it possibly runs so fast that it always finishes before the next task starts running). https://github.com/llvm/llvm-project/pull/93977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [OMPIRBuilder] - Handle dependencies in `createTarget` (PR #93977)
@@ -5272,36 +5682,53 @@ static void emitTargetCall(OpenMPIRBuilder &OMPBuilder, IRBuilderBase &Builder, Value *DynCGGroupMem = Builder.getInt32(0); bool HasNoWait = false; + bool HasDependencies = Dependencies.size() > 0; + bool RequiresOuterTargetTask = HasNoWait || HasDependencies; OpenMPIRBuilder::TargetKernelArgs KArgs(NumTargetItems, RTArgs, NumIterations, NumTeamsVal, NumThreadsVal, DynCGGroupMem, HasNoWait); - Builder.restoreIP(OMPBuilder.emitKernelLaunch( - Builder, OutlinedFn, OutlinedFnID, EmitTargetCallFallbackCB, KArgs, - DeviceID, RTLoc, AllocaIP)); + // The presence of certain clauses on the target directive require the + // explicit generation of the target task. + if (RequiresOuterTargetTask) { +OMPBuilder.emitTargetTask(OutlinedFn, OutlinedFnID, skatrak wrote: Is a call to `Builder.restoreIP()` missing? https://github.com/llvm/llvm-project/pull/93977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [OMPIRBuilder] - Handle dependencies in `createTarget` (PR #93977)
@@ -792,6 +792,9 @@ void OpenMPIRBuilder::finalize(Function *Fn) { if (!OffloadInfoManager.empty()) createOffloadEntriesAndInfoMetadata(ErrorReportFn); + + LLVM_DEBUG(dbgs() << "Module after OMPIRBuilder::finalize\n"); + LLVM_DEBUG(dbgs() << M << "\n"); skatrak wrote: Nit: Is this addition intentional? https://github.com/llvm/llvm-project/pull/93977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [OMPIRBuilder] - Handle dependencies in `createTarget` (PR #93977)
@@ -1698,6 +1701,64 @@ void OpenMPIRBuilder::createTaskyield(const LocationDescription &Loc) { emitTaskyieldImpl(Loc); } +// Processes the dependencies in Dependencies and does the following +// - Allocates space on the stack of an array of DependInfo objects +// - Populates each DependInfo object with relevant information of +// the corresponding dependence. +// - All code is inserted in the entry block of the current function. +static Value * +emitDepArray(OpenMPIRBuilder &OMPBuilder, skatrak wrote: ```suggestion emitTargetTaskDependenciesArray(OpenMPIRBuilder &OMPBuilder, ``` Nit: I think it's best to try avoiding arbitrary abbreviations in function names for readability. Perhaps the name I'm suggesting can be shortened if it's also intended for general tasks (`emitTaskDependenciesArray`), but it looks like it's only used for target tasks at the moment. https://github.com/llvm/llvm-project/pull/93977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [OMPIRBuilder] - Handle dependencies in `createTarget` (PR #93977)
@@ -705,28 +728,9 @@ convertOmpTaskOp(omp::TaskOp taskOp, llvm::IRBuilderBase &builder, }; SmallVector dds; - if (!taskOp.getDependVars().empty() && taskOp.getDepends()) { -for (auto dep : - llvm::zip(taskOp.getDependVars(), taskOp.getDepends()->getValue())) { - llvm::omp::RTLDependenceKindTy type; - switch ( - cast(std::get<1>(dep)).getValue()) { - case mlir::omp::ClauseTaskDepend::taskdependin: -type = llvm::omp::RTLDependenceKindTy::DepIn; -break; - // The OpenMP runtime requires that the codegen for 'depend' clause for - // 'out' dependency kind must be the same as codegen for 'depend' clause - // with 'inout' dependency. - case mlir::omp::ClauseTaskDepend::taskdependout: - case mlir::omp::ClauseTaskDepend::taskdependinout: -type = llvm::omp::RTLDependenceKindTy::DepInOut; -break; - }; - llvm::Value *depVal = moduleTranslation.lookupValue(std::get<0>(dep)); - llvm::OpenMPIRBuilder::DependData dd(type, depVal->getType(), depVal); - dds.emplace_back(dd); -} - } + if (!taskOp.getDependVars().empty() && taskOp.getDepends()) skatrak wrote: Nit: I'd suggest moving this check into `buildDependData`, since it always needs to be done. It would just return without adding anything to `dds` if the condition is not met. https://github.com/llvm/llvm-project/pull/93977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][OpenMP] Add -fopenmp-force-usm option to flang (PR #94359)
https://github.com/skatrak updated https://github.com/llvm/llvm-project/pull/94359 >From 88a2553168b4fd3ad1b65b855c2bdf5aba09d126 Mon Sep 17 00:00:00 2001 From: Sergio Afonso Date: Tue, 4 Jun 2024 15:26:38 +0100 Subject: [PATCH 1/2] [Flang][OpenMP] Add -fopenmp-force-usm option to flang This patch enables the `-fopenmp-force-usm` option to be passed to the flang driver, which forwards it to the compiler frontend. This flag, when set, results in the introduction of the `unified_shared_memory` bit to the `omp.requires` attribute of the top-level module operation. This is later combined with any other target device-related REQUIRES clauses that may have been explicitly set in the compilation unit. --- clang/include/clang/Driver/Options.td | 2 +- clang/lib/Driver/ToolChains/Flang.cpp | 2 ++ flang/include/flang/Frontend/LangOptions.def | 2 ++ flang/include/flang/Tools/CrossToolHelpers.h | 20 --- flang/lib/Frontend/CompilerInvocation.cpp | 3 +++ flang/lib/Lower/OpenMP/OpenMP.cpp | 4 +++- flang/test/Driver/omp-driver-offload.f90 | 20 +++ flang/test/Lower/OpenMP/force-usm.f90 | 12 +++ .../test/Lower/OpenMP/requires-force-usm.f90 | 15 ++ flang/tools/bbc/bbc.cpp | 15 +- 10 files changed, 81 insertions(+), 14 deletions(-) create mode 100644 flang/test/Lower/OpenMP/force-usm.f90 create mode 100644 flang/test/Lower/OpenMP/requires-force-usm.f90 diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 5ab2d49c7a497..f04c220d6e1db 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3592,7 +3592,7 @@ def fopenmp_offload_mandatory : Flag<["-"], "fopenmp-offload-mandatory">, Group< HelpText<"Do not create a host fallback if offloading to the device fails.">, MarshallingInfoFlag>; def fopenmp_force_usm : Flag<["-"], "fopenmp-force-usm">, Group, - Flags<[NoArgumentUnused]>, Visibility<[ClangOption, CC1Option]>, + Flags<[NoArgumentUnused]>, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>, HelpText<"Force behvaior as if the user specified pragma omp requires unified_shared_memory.">, MarshallingInfoFlag>; def fopenmp_target_jit : Flag<["-"], "fopenmp-target-jit">, Group, diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp index 42ca060186fd8..b7abe7b1c19bc 100644 --- a/clang/lib/Driver/ToolChains/Flang.cpp +++ b/clang/lib/Driver/ToolChains/Flang.cpp @@ -766,6 +766,8 @@ void Flang::ConstructJob(Compilation &C, const JobAction &JA, Args.AddAllArgs(CmdArgs, options::OPT_fopenmp_version_EQ); // FIXME: Clang supports a whole bunch more flags here. + if (Args.hasArg(options::OPT_fopenmp_force_usm)) +CmdArgs.push_back("-fopenmp-force-usm"); break; default: // By default, if Clang doesn't know how to generate useful OpenMP code diff --git a/flang/include/flang/Frontend/LangOptions.def b/flang/include/flang/Frontend/LangOptions.def index 2bf10826120a8..d3e1e972d1519 100644 --- a/flang/include/flang/Frontend/LangOptions.def +++ b/flang/include/flang/Frontend/LangOptions.def @@ -42,6 +42,8 @@ LANGOPT(OpenMPVersion, 32, 0) LANGOPT(OpenMPIsTargetDevice, 1, false) /// Generate OpenMP target code only for GPUs LANGOPT(OpenMPIsGPU, 1, false) +/// Generate OpenMP target code only for GPUs +LANGOPT(OpenMPForceUSM, 1, false) /// Enable debugging in the OpenMP offloading device RTL LANGOPT(OpenMPTargetDebug, 32, 0) /// Assume work-shared loops do not have more iterations than participating diff --git a/flang/include/flang/Tools/CrossToolHelpers.h b/flang/include/flang/Tools/CrossToolHelpers.h index 77b68fc6187fa..26fbe51d329c7 100644 --- a/flang/include/flang/Tools/CrossToolHelpers.h +++ b/flang/include/flang/Tools/CrossToolHelpers.h @@ -130,16 +130,16 @@ struct OffloadModuleOpts { OffloadModuleOpts(uint32_t OpenMPTargetDebug, bool OpenMPTeamSubscription, bool OpenMPThreadSubscription, bool OpenMPNoThreadState, bool OpenMPNoNestedParallelism, bool OpenMPIsTargetDevice, - bool OpenMPIsGPU, uint32_t OpenMPVersion, std::string OMPHostIRFile = {}, - bool NoGPULib = false) + bool OpenMPIsGPU, bool OpenMPForceUSM, uint32_t OpenMPVersion, + std::string OMPHostIRFile = {}, bool NoGPULib = false) : OpenMPTargetDebug(OpenMPTargetDebug), OpenMPTeamSubscription(OpenMPTeamSubscription), OpenMPThreadSubscription(OpenMPThreadSubscription), OpenMPNoThreadState(OpenMPNoThreadState), OpenMPNoNestedParallelism(OpenMPNoNestedParallelism), OpenMPIsTargetDevice(OpenMPIsTargetDevice), OpenMPIsGPU(OpenMPIsGPU), -OpenMPVersion(OpenMPVersion), OMPHostIRFile(OMPHostIRFile), -NoGPULib(NoGPULib) {} +OpenMPForceUSM(OpenMPForceUSM), OpenMPVersion(OpenMPVersion), +OMPHostIRFile(OMPHostIRFile
[clang] [flang] [Flang][OpenMP] Add -fopenmp-force-usm option to flang (PR #94359)
skatrak wrote: Thank your for the reviews. Merging now since buildbot failure is unrelated. https://github.com/llvm/llvm-project/pull/94359 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][OpenMP] Add -fopenmp-force-usm option to flang (PR #94359)
https://github.com/skatrak closed https://github.com/llvm/llvm-project/pull/94359 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [OMPIRBuilder] - Handle dependencies in `createTarget` (PR #93977)
https://github.com/skatrak commented: Thanks for working on my previous comments. I just have a couple more minor ones. https://github.com/llvm/llvm-project/pull/93977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [OMPIRBuilder] - Handle dependencies in `createTarget` (PR #93977)
@@ -5272,36 +5682,53 @@ static void emitTargetCall(OpenMPIRBuilder &OMPBuilder, IRBuilderBase &Builder, Value *DynCGGroupMem = Builder.getInt32(0); bool HasNoWait = false; + bool HasDependencies = Dependencies.size() > 0; + bool RequiresOuterTargetTask = HasNoWait || HasDependencies; OpenMPIRBuilder::TargetKernelArgs KArgs(NumTargetItems, RTArgs, NumIterations, NumTeamsVal, NumThreadsVal, DynCGGroupMem, HasNoWait); - Builder.restoreIP(OMPBuilder.emitKernelLaunch( - Builder, OutlinedFn, OutlinedFnID, EmitTargetCallFallbackCB, KArgs, - DeviceID, RTLoc, AllocaIP)); + // The presence of certain clauses on the target directive require the + // explicit generation of the target task. + if (RequiresOuterTargetTask) { +OMPBuilder.emitTargetTask(OutlinedFn, OutlinedFnID, skatrak wrote: I agree with you, since I have noticed this redundant pattern as well, though in a way I think it makes sense from a uniformity point of view. There are several codegen functions that return an insertion point, which should be used by the caller to proceed with any further codegen. I think that the fact that oftentimes the insertion point that's being returned is indeed already the current one shouldn't be relied on by the caller unless we make this a contract followed by all such functions. To be consistent, I think these codegen function must either always return an insertion point which must be used by the caller (the current approach) or all codegen functions must not return an insertion point and set it themselves to the spot follow-up codegen should go. In my opinion, I believe it's currently better to keep the current approach even though it results in some redundant setting of the insertion point. Skipping the call to `Builder.restoreIP()` in the caller could result in unintended problems in the future, since the current contract is that the returned insertion point is the one that should be used and not the current one. https://github.com/llvm/llvm-project/pull/93977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [OMPIRBuilder] - Handle dependencies in `createTarget` (PR #93977)
@@ -5229,13 +5382,284 @@ static void emitTargetOutlinedFunction( OMPBuilder.emitTargetRegionFunction(EntryInfo, GenerateOutlinedFunction, true, OutlinedFn, OutlinedFnID); } +OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::emitTargetTask( +Function *OutlinedFn, Value *OutlinedFnID, +EmitFallbackCallbackTy EmitTargetCallFallbackCB, TargetKernelArgs &Args, +Value *DeviceID, Value *RTLoc, OpenMPIRBuilder::InsertPointTy AllocaIP, +SmallVector &Dependencies, +bool HasNoWait) { + + // When we arrive at this function, the target region itself has been + // outlined into the function OutlinedFn. + // So at ths point, for + // -- + // void user_code_that_offloads(...) { + // omp target depend(..) map(from:a) map(to:b, c) + //a = b + c + // } + // + // -- + // + // we have + // + // -- + // + // void user_code_that_offloads(...) { + // %.offload_baseptrs = alloca [3 x ptr], align 8 + // %.offload_ptrs = alloca [3 x ptr], align 8 + // %.offload_mappers = alloca [3 x ptr], align 8 + // ;; target region has been outlined and now we need to + // ;; offload to it via a target task. + // } + // void outlined_device_function(ptr a, ptr b, ptr c) { + // *a = *b + *c + // } + // + // We have to now do the following + // (i) Make an offloading call to outlined_device_function using the OpenMP + // RTL. See 'kernel_launch_function' in the pseudo code below. This is + // emitted by emitKernelLaunch + // (ii) Create a task entry point function that calls kernel_launch_function + // and is the entry point for the target task. See + // '@.omp_target_task_proxy_func in the pseudocode below. + // (iii) Create a task with the task entry point created in (ii) + // + // That is we create the following + // + // void user_code_that_offloads(...) { + // %.offload_baseptrs = alloca [3 x ptr], align 8 + // %.offload_ptrs = alloca [3 x ptr], align 8 + // %.offload_mappers = alloca [3 x ptr], align 8 + // + // %structArg = alloca { ptr, ptr, ptr }, align 8 + // %strucArg[0] = %.offload_baseptrs + // %strucArg[1] = %.offload_ptrs + // %strucArg[2] = %.offload_mappers + // proxy_target_task = @__kmpc_omp_task_alloc(..., + // @.omp_target_task_proxy_func) + // memcpy(proxy_target_task->shareds, %structArg, sizeof(structArg)) + // dependencies_array = ... + // ;; if nowait not present + // call @__kmpc_omp_wait_deps(..., dependencies_array) + // call @__kmpc_omp_task_begin_if0(...) + // call @ @.omp_target_task_proxy_func(i32 thread_id, ptr + // %proxy_target_task) call @__kmpc_omp_task_complete_if0(...) + // } + // + // define internal void @.omp_target_task_proxy_func(i32 %thread.id, + // ptr %task) { + // %structArg = alloca {ptr, ptr, ptr} + // %shared_data = load (getelementptr %task, 0, 0) + // mempcy(%structArg, %shared_data, sizeof(structArg)) + // kernel_launch_function(%thread.id, %structArg) + // } + // + // We need the proxy function because the signature of the task entry point + // expected by kmpc_omp_task is always the same and will be different from + // that of the kernel_launch function. + // + // kernel_launch_function is generated by emitKernelLaunch and has the + // always_inline attribute. + // void kernel_launch_function(thread_id, + // structArg) alwaysinline { + // %kernel_args = alloca %struct.__tgt_kernel_arguments, align 8 + // offload_baseptrs = load(getelementptr structArg, 0, 0) + // offload_ptrs = load(getelementptr structArg, 0, 1) + // offload_mappers = load(getelementptr structArg, 0, 2) + // ; setup kernel_args using offload_baseptrs, offload_ptrs and + // ; offload_mappers + // call i32 @__tgt_target_kernel(..., + // outlined_device_function, + // ptr %kernel_args) + // } + // void outlined_device_function(ptr a, ptr b, ptr c) { + // *a = *b + *c + // } + // + BasicBlock *TargetTaskBodyBB = + splitBB(Builder, /*CreateBranch=*/true, "target.task.body"); + BasicBlock *TargetTaskAllocaBB = + splitBB(Builder, /*CreateBranch=*/true, "target.task.alloca"); + + InsertPointTy TargetTaskAllocaIP = + InsertPointTy(TargetTaskAllocaBB, TargetTaskAllocaBB->begin()); + InsertPointTy TargetTaskBodyIP = + InsertPointTy(TargetTaskBodyBB, TargetTaskBodyBB->begin()); + + OutlineInfo OI; + OI.EntryBB = TargetTaskAllocaBB; + OI.OuterAllocaBB = AllocaIP.getBlock(); + + //
[clang] [llvm] [mlir] [OMPIRBuilder] - Handle dependencies in `createTarget` (PR #93977)
https://github.com/skatrak edited https://github.com/llvm/llvm-project/pull/93977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [OMPIRBuilder] - Handle dependencies in `createTarget` (PR #93977)
@@ -0,0 +1,83 @@ +! Offloading test checking the use of the depend clause on +! the target construct +! REQUIRES: flang, amdgcn-amd-amdhsa +! UNSUPPORTED: nvptx64-nvidia-cuda +! UNSUPPORTED: nvptx64-nvidia-cuda-LTO +! UNSUPPORTED: aarch64-unknown-linux-gnu +! UNSUPPORTED: aarch64-unknown-linux-gnu-LTO +! UNSUPPORTED: x86_64-pc-linux-gnu +! UNSUPPORTED: x86_64-pc-linux-gnu-LTO + +! RUN: %libomptarget-compile-fortran-run-and-check-generic +program main + implicit none + integer :: a = 0 + INTERFACE + FUNCTION omp_get_device_num() BIND(C) + USE, INTRINSIC :: iso_c_binding, ONLY: C_INT + integer :: omp_get_device_num + END FUNCTION omp_get_device_num + END INTERFACE + + call foo(5, a) + print*, "=== FORTRAN Test passed! ===" + print*, "foo(5) returned ", a, ", expected 6\n" + + ! stop 0 + contains +subroutine foo(N, r) + integer, intent(in) :: N + integer, intent(out) :: r + integer :: z, i, j, k, accumulator + z = 1 + accumulator = 0 + ! Spawn 3 threads + !$omp parallel num_threads(3) + + ! A single thread will then create two tasks + ! One is the 'producer' and potentially slower + ! task that updates 'z' to 'N'. The second is an + ! offloaded target task that increments 'z'. + ! If the depend clauses work properly, the + ! target task should wait for the 'producer' + ! task to complete before incrementing z + ! We use !$omp single here because only + ! the depend clause establishes dependencies + ! between sibling tasks only. This is the easiest + ! way of creating two sibling tasks. + !$omp single + !$omp task depend(out: z) shared(z) + do while (k < 32000) + do while (j < 32766) +do while (i < 32766) + ! dumb loop nest to slow down the update of + ! z + i = i + 1 + ! Adding a function call slows down the producer + ! to the point that removing the depend clause + ! from the target construct below frequently + ! results in the wrong answer. + accumulator = accumulator + omp_get_device_num() +end do +j = j +1 + end do + k = k + 1 + end do + z = N + !$omp end task + + ! z is 5 now. Increment z to 6. + !$omp target map(tofrom: z) depend(in:z) + z = z + 1 + !$omp end target + !$omp end single + !$omp end parallel + ! Use 'accumulator' so it is not optimized away + ! by the compiler. + print *, accumulator + r = z +end subroutine foo skatrak wrote: ```suggestion end subroutine foo ``` https://github.com/llvm/llvm-project/pull/93977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [OMPIRBuilder] - Handle dependencies in `createTarget` (PR #93977)
@@ -0,0 +1,83 @@ +! Offloading test checking the use of the depend clause on +! the target construct +! REQUIRES: flang, amdgcn-amd-amdhsa +! UNSUPPORTED: nvptx64-nvidia-cuda +! UNSUPPORTED: nvptx64-nvidia-cuda-LTO +! UNSUPPORTED: aarch64-unknown-linux-gnu +! UNSUPPORTED: aarch64-unknown-linux-gnu-LTO +! UNSUPPORTED: x86_64-pc-linux-gnu +! UNSUPPORTED: x86_64-pc-linux-gnu-LTO + +! RUN: %libomptarget-compile-fortran-run-and-check-generic +program main + implicit none + integer :: a = 0 + INTERFACE + FUNCTION omp_get_device_num() BIND(C) + USE, INTRINSIC :: iso_c_binding, ONLY: C_INT + integer :: omp_get_device_num + END FUNCTION omp_get_device_num + END INTERFACE + + call foo(5, a) + print*, "=== FORTRAN Test passed! ===" + print*, "foo(5) returned ", a, ", expected 6\n" + + ! stop 0 + contains +subroutine foo(N, r) + integer, intent(in) :: N + integer, intent(out) :: r + integer :: z, i, j, k, accumulator + z = 1 + accumulator = 0 + ! Spawn 3 threads + !$omp parallel num_threads(3) + + ! A single thread will then create two tasks + ! One is the 'producer' and potentially slower + ! task that updates 'z' to 'N'. The second is an + ! offloaded target task that increments 'z'. + ! If the depend clauses work properly, the + ! target task should wait for the 'producer' + ! task to complete before incrementing z + ! We use !$omp single here because only + ! the depend clause establishes dependencies + ! between sibling tasks only. This is the easiest + ! way of creating two sibling tasks. + !$omp single + !$omp task depend(out: z) shared(z) + do while (k < 32000) skatrak wrote: Couldn't this loop be written more concisely as `do k=1, 32000 ... end do`? Same for the other two loops nested in it. https://github.com/llvm/llvm-project/pull/93977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [OMPIRBuilder] - Handle dependencies in `createTarget` (PR #93977)
@@ -705,28 +728,9 @@ convertOmpTaskOp(omp::TaskOp taskOp, llvm::IRBuilderBase &builder, }; SmallVector dds; - if (!taskOp.getDependVars().empty() && taskOp.getDepends()) { -for (auto dep : - llvm::zip(taskOp.getDependVars(), taskOp.getDepends()->getValue())) { - llvm::omp::RTLDependenceKindTy type; - switch ( - cast(std::get<1>(dep)).getValue()) { - case mlir::omp::ClauseTaskDepend::taskdependin: -type = llvm::omp::RTLDependenceKindTy::DepIn; -break; - // The OpenMP runtime requires that the codegen for 'depend' clause for - // 'out' dependency kind must be the same as codegen for 'depend' clause - // with 'inout' dependency. - case mlir::omp::ClauseTaskDepend::taskdependout: - case mlir::omp::ClauseTaskDepend::taskdependinout: -type = llvm::omp::RTLDependenceKindTy::DepInOut; -break; - }; - llvm::Value *depVal = moduleTranslation.lookupValue(std::get<0>(dep)); - llvm::OpenMPIRBuilder::DependData dd(type, depVal->getType(), depVal); - dds.emplace_back(dd); -} - } + if (!taskOp.getDependVars().empty() && taskOp.getDepends()) skatrak wrote: It looks like this comment hasn't been addressed. Let me know if you just missed it, if you disagree with it or if it isn't very clear and I should try to explain better. https://github.com/llvm/llvm-project/pull/93977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [OMPIRBuilder] - Handle dependencies in `createTarget` (PR #93977)
@@ -0,0 +1,83 @@ +! Offloading test checking the use of the depend clause on +! the target construct +! REQUIRES: flang, amdgcn-amd-amdhsa +! UNSUPPORTED: nvptx64-nvidia-cuda +! UNSUPPORTED: nvptx64-nvidia-cuda-LTO +! UNSUPPORTED: aarch64-unknown-linux-gnu +! UNSUPPORTED: aarch64-unknown-linux-gnu-LTO +! UNSUPPORTED: x86_64-pc-linux-gnu +! UNSUPPORTED: x86_64-pc-linux-gnu-LTO + +! RUN: %libomptarget-compile-fortran-run-and-check-generic +program main + implicit none + integer :: a = 0 + INTERFACE + FUNCTION omp_get_device_num() BIND(C) + USE, INTRINSIC :: iso_c_binding, ONLY: C_INT + integer :: omp_get_device_num + END FUNCTION omp_get_device_num + END INTERFACE + + call foo(5, a) + print*, "=== FORTRAN Test passed! ===" + print*, "foo(5) returned ", a, ", expected 6\n" + + ! stop 0 + contains +subroutine foo(N, r) + integer, intent(in) :: N + integer, intent(out) :: r + integer :: z, i, j, k, accumulator + z = 1 + accumulator = 0 + ! Spawn 3 threads + !$omp parallel num_threads(3) + + ! A single thread will then create two tasks skatrak wrote: Nit: It looks like comments in this file are split very early on, can you extend to 80 characters? https://github.com/llvm/llvm-project/pull/93977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 094a63a - [OpenMP][OMPIRBuilder] OpenMPIRBuilder support for requires directive
Author: Sergio Afonso Date: 2023-09-14T10:33:54+01:00 New Revision: 094a63a20bf54f18efafbb6a727466da0f22a245 URL: https://github.com/llvm/llvm-project/commit/094a63a20bf54f18efafbb6a727466da0f22a245 DIFF: https://github.com/llvm/llvm-project/commit/094a63a20bf54f18efafbb6a727466da0f22a245.diff LOG: [OpenMP][OMPIRBuilder] OpenMPIRBuilder support for requires directive This patch updates the `OpenMPIRBuilderConfig` structure to hold all available 'requires' clauses, and it replicates part of the code generation for the 'requires' registration function from clang in the `OMPIRBuilder`, to be used with flang. Porting the rest of features of the clang implementation to the IRBuilder and sharing it between clang and flang remains for a future patch, due to the complexity of the logic selecting the attributes of the generated registration function. Differential Revision: https://reviews.llvm.org/D147217 Added: Modified: clang/lib/CodeGen/CGOpenMPRuntime.cpp clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp mlir/lib/Target/LLVMIR/ModuleTranslation.cpp Removed: diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp b/clang/lib/CodeGen/CGOpenMPRuntime.cpp index 380bab65c14f2e6..3b69305d050d769 100644 --- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp +++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp @@ -480,27 +480,6 @@ enum OpenMPLocationFlags : unsigned { LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/OMP_IDENT_WORK_DISTRIBUTE) }; -namespace { -LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE(); -/// Values for bit flags for marking which requires clauses have been used. -enum OpenMPOffloadingRequiresDirFlags : int64_t { - /// flag undefined. - OMP_REQ_UNDEFINED = 0x000, - /// no requires clause present. - OMP_REQ_NONE= 0x001, - /// reverse_offload clause. - OMP_REQ_REVERSE_OFFLOAD = 0x002, - /// unified_address clause. - OMP_REQ_UNIFIED_ADDRESS = 0x004, - /// unified_shared_memory clause. - OMP_REQ_UNIFIED_SHARED_MEMORY = 0x008, - /// dynamic_allocators clause. - OMP_REQ_DYNAMIC_ALLOCATORS = 0x010, - LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/OMP_REQ_DYNAMIC_ALLOCATORS) -}; - -} // anonymous namespace - /// Describes ident structure that describes a source location. /// All descriptions are taken from /// https://github.com/llvm/llvm-project/blob/main/openmp/runtime/src/kmp.h @@ -1055,9 +1034,11 @@ static FieldDecl *addFieldToRecordDecl(ASTContext &C, DeclContext *DC, CGOpenMPRuntime::CGOpenMPRuntime(CodeGenModule &CGM) : CGM(CGM), OMPBuilder(CGM.getModule()) { KmpCriticalNameTy = llvm::ArrayType::get(CGM.Int32Ty, /*NumElements*/ 8); - llvm::OpenMPIRBuilderConfig Config(CGM.getLangOpts().OpenMPIsTargetDevice, - isGPU(), hasRequiresUnifiedSharedMemory(), - CGM.getLangOpts().OpenMPOffloadMandatory); + llvm::OpenMPIRBuilderConfig Config( + CGM.getLangOpts().OpenMPIsTargetDevice, isGPU(), + CGM.getLangOpts().OpenMPOffloadMandatory, + /*HasRequiresReverseOffload*/ false, /*HasRequiresUnifiedAddress*/ false, + hasRequiresUnifiedSharedMemory(), /*HasRequiresDynamicAllocators*/ false); OMPBuilder.initialize(CGM.getLangOpts().OpenMPIsTargetDevice ? CGM.getLangOpts().OMPHostIRFile : StringRef{}); @@ -10162,7 +10143,6 @@ llvm::Function *CGOpenMPRuntime::emitRequiresDirectiveRegFun() { std::string ReqName = getName({"omp_offloading", "requires_reg"}); RequiresRegFn = CGM.CreateGlobalInitOrCleanUpFunction(FTy, ReqName, FI); CGF.StartFunction(GlobalDecl(), C.VoidTy, RequiresRegFn, FI, {}); -OpenMPOffloadingRequiresDirFlags Flags = OMP_REQ_NONE; // TODO: check for other requires clauses. // The requires directive takes effect only when a target region is // present in the compilation unit. Otherwise it is ignored and not @@ -10172,11 +10152,10 @@ llvm::Function *CGOpenMPRuntime::emitRequiresDirectiveRegFun() { assert((HasEmittedTargetRegion || HasEmittedDeclareTargetRegion || !OMPBuilder.OffloadInfoManager.empty()) && "Target or declare target region expected."); -if (HasRequiresUnifiedSharedMemory) - Flags = OMP_REQ_UNIFIED_SHARED_MEMORY; CGF.EmitRuntimeCall(OMPBuilder.getOrCreateRuntimeFunction( CGM.getModule(), OMPRTL___tgt_register_requires), -llvm::ConstantInt::get(CGM.Int64Ty, Flags)); +llvm::ConstantInt::get( +CGM.Int64Ty, OMPBuilder.Config.getRequiresFlags())); CGF.FinishFunction(); } return RequiresRegFn; diff --git a/clang/lib/CodeGen/CGOpenMPRuntimeGPU.c
[clang] 9058762 - [OpenMP][Flang][MLIR] Lowering of requires directive from MLIR to LLVM IR
Author: Sergio Afonso Date: 2023-09-14T10:35:44+01:00 New Revision: 9058762789c0a83560c2b567a347b993e70b05ae URL: https://github.com/llvm/llvm-project/commit/9058762789c0a83560c2b567a347b993e70b05ae DIFF: https://github.com/llvm/llvm-project/commit/9058762789c0a83560c2b567a347b993e70b05ae.diff LOG: [OpenMP][Flang][MLIR] Lowering of requires directive from MLIR to LLVM IR Default atomic ordering information is processed in the OpenMP dialect to LLVM IR lowering stage at every spot where an operation can be affected by it. The rest of clauses are stored globally in the OpenMPIRBuilderConfig object before starting that lowering stage, so that the OMPIRBuilder can conditionally modify code generation depending on these. At the end of the process, the omp.requires attribute is itself lowered into a global constructor that passes these clauses as flags to the OpenMP runtime. Depends on D147217, D147218 and D158278. Differential Revision: https://reviews.llvm.org/D147219 Added: Modified: clang/lib/CodeGen/CGOpenMPRuntime.cpp llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp mlir/lib/Target/LLVMIR/ModuleTranslation.cpp mlir/test/Target/LLVMIR/openmp-llvm.mlir Removed: diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp b/clang/lib/CodeGen/CGOpenMPRuntime.cpp index 3b69305d050d769..92b7c8d4aa546f0 100644 --- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp +++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp @@ -1039,9 +1039,10 @@ CGOpenMPRuntime::CGOpenMPRuntime(CodeGenModule &CGM) CGM.getLangOpts().OpenMPOffloadMandatory, /*HasRequiresReverseOffload*/ false, /*HasRequiresUnifiedAddress*/ false, hasRequiresUnifiedSharedMemory(), /*HasRequiresDynamicAllocators*/ false); - OMPBuilder.initialize(CGM.getLangOpts().OpenMPIsTargetDevice -? CGM.getLangOpts().OMPHostIRFile -: StringRef{}); + OMPBuilder.initialize(); + OMPBuilder.loadOffloadInfoMetadata(CGM.getLangOpts().OpenMPIsTargetDevice + ? CGM.getLangOpts().OMPHostIRFile + : StringRef{}); OMPBuilder.setConfig(Config); } diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h index a5977356da3e530..523a0718e1ffb53 100644 --- a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h +++ b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h @@ -154,6 +154,7 @@ class OpenMPIRBuilderConfig { void setIsTargetDevice(bool Value) { IsTargetDevice = Value; } void setIsGPU(bool Value) { IsGPU = Value; } + void setOpenMPOffloadMandatory(bool Value) { OpenMPOffloadMandatory = Value; } void setFirstSeparator(StringRef FS) { FirstSeparator = FS; } void setSeparator(StringRef S) { Separator = S; } @@ -449,14 +450,9 @@ class OpenMPIRBuilder { /// Initialize the internal state, this will put structures types and /// potentially other helpers into the underlying module. Must be called - /// before any other method and only once! This internal state includes - /// Types used in the OpenMPIRBuilder generated from OMPKinds.def as well - /// as loading offload metadata for device from the OpenMP host IR file - /// passed in as the HostFilePath argument. - /// \param HostFilePath The path to the host IR file, used to load in - /// offload metadata for the device, allowing host and device to - /// maintain the same metadata mapping. - void initialize(StringRef HostFilePath = {}); + /// before any other method and only once! This internal state includes types + /// used in the OpenMPIRBuilder generated from OMPKinds.def. + void initialize(); void setConfig(OpenMPIRBuilderConfig C) { Config = C; } @@ -2519,6 +2515,15 @@ class OpenMPIRBuilder { /// loaded from bitcode file, i.e, diff erent from OpenMPIRBuilder::M module. void loadOffloadInfoMetadata(Module &M); + /// Loads all the offload entries information from the host IR + /// metadata read from the file passed in as the HostFilePath argument. This + /// function is only meant to be used with device code generation. + /// + /// \param HostFilePath The path to the host IR file, + /// used to load in offload metadata for the device, allowing host and device + /// to maintain the same metadata mapping. + void loadOffloadInfoMetadata(StringRef HostFilePath); + /// Gets (if variable with the given name already exist) or creates /// internal global variable with the specified Name. The created variable has /// linkage CommonLinkage by default and is initialized by null value. diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp index cc6054cb19d7806..9341c529c603e5d 100644 --- a/llvm/lib/Frontend/
[clang] ec70627 - [Flang][Sema] Move directive sets to a shared location
Author: Sergio Afonso Date: 2023-08-07T11:18:43+01:00 New Revision: ec70627dd17703a2a12ce0f28bd3794aa77d2058 URL: https://github.com/llvm/llvm-project/commit/ec70627dd17703a2a12ce0f28bd3794aa77d2058 DIFF: https://github.com/llvm/llvm-project/commit/ec70627dd17703a2a12ce0f28bd3794aa77d2058.diff LOG: [Flang][Sema] Move directive sets to a shared location This patch moves directive sets defined internally in Semantics to a header accessible by other stages of the compiler to enable reuse. Some sets are renamed/rearranged and others are lifted from local definitions to provide a single source of truth. Differential Revision: https://reviews.llvm.org/D157090 Added: flang/include/flang/Semantics/openmp-directive-sets.h Modified: clang/docs/tools/clang-formatted-files.txt flang/lib/Semantics/check-omp-structure.cpp flang/lib/Semantics/check-omp-structure.h flang/lib/Semantics/resolve-directives.cpp Removed: diff --git a/clang/docs/tools/clang-formatted-files.txt b/clang/docs/tools/clang-formatted-files.txt index 27aab1e1305245..e22f01e8c150e1 100644 --- a/clang/docs/tools/clang-formatted-files.txt +++ b/clang/docs/tools/clang-formatted-files.txt @@ -2185,6 +2185,8 @@ flang/include/flang/Runtime/time-intrinsic.h flang/include/flang/Runtime/transformational.h flang/include/flang/Runtime/type-code.h flang/include/flang/Semantics/attr.h +flang/include/flang/Semantics/expression.h +flang/include/flang/Semantics/openmp-directive-sets.h flang/include/flang/Semantics/runtime-type-info.h flang/include/flang/Semantics/scope.h flang/include/flang/Semantics/semantics.h diff --git a/flang/include/flang/Semantics/openmp-directive-sets.h b/flang/include/flang/Semantics/openmp-directive-sets.h new file mode 100644 index 00..5ccd75842b878e --- /dev/null +++ b/flang/include/flang/Semantics/openmp-directive-sets.h @@ -0,0 +1,283 @@ +//===-- include/flang/Semantics/openmp-directive-sets.h -*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#ifndef FORTRAN_SEMANTICS_OPENMP_DIRECTIVE_SETS_H_ +#define FORTRAN_SEMANTICS_OPENMP_DIRECTIVE_SETS_H_ + +#include "flang/Common/enum-set.h" +#include "llvm/Frontend/OpenMP/OMPConstants.h" + +using OmpDirectiveSet = Fortran::common::EnumSet; + +namespace llvm::omp { +//===--===// +// Directive sets for single directives +//===--===// +// - topSet: The directive appears alone or as the first in a +// combined construct. +// - allSet: All standalone or combined uses of the directive. + +const inline OmpDirectiveSet topParallelSet{ +Directive::OMPD_parallel, +Directive::OMPD_parallel_do, +Directive::OMPD_parallel_do_simd, +Directive::OMPD_parallel_sections, +Directive::OMPD_parallel_workshare, +}; + +const inline OmpDirectiveSet allParallelSet{ +Directive::OMPD_distribute_parallel_do, +Directive::OMPD_distribute_parallel_do_simd, +Directive::OMPD_parallel, +Directive::OMPD_parallel_do, +Directive::OMPD_parallel_do_simd, +Directive::OMPD_parallel_sections, +Directive::OMPD_parallel_workshare, +Directive::OMPD_target_parallel, +Directive::OMPD_target_parallel_do, +Directive::OMPD_target_parallel_do_simd, +Directive::OMPD_target_teams_distribute_parallel_do, +Directive::OMPD_target_teams_distribute_parallel_do_simd, +Directive::OMPD_teams_distribute_parallel_do, +Directive::OMPD_teams_distribute_parallel_do_simd, +}; + +const inline OmpDirectiveSet topDoSet{ +Directive::OMPD_do, +Directive::OMPD_do_simd, +}; + +const inline OmpDirectiveSet allDoSet{ +Directive::OMPD_distribute_parallel_do, +Directive::OMPD_distribute_parallel_do_simd, +Directive::OMPD_parallel_do, +Directive::OMPD_parallel_do_simd, +Directive::OMPD_do, +Directive::OMPD_do_simd, +Directive::OMPD_target_parallel_do, +Directive::OMPD_target_parallel_do_simd, +Directive::OMPD_target_teams_distribute_parallel_do, +Directive::OMPD_target_teams_distribute_parallel_do_simd, +Directive::OMPD_teams_distribute_parallel_do, +Directive::OMPD_teams_distribute_parallel_do_simd, +}; + +const inline OmpDirectiveSet topTaskloopSet{ +Directive::OMPD_taskloop, +Directive::OMPD_taskloop_simd, +}; + +const inline OmpDirectiveSet allTaskloopSet = topTaskloopSet; + +const inline OmpDirectiveSet topTargetSet{ +Directive::OMPD_target, +Directive::OMPD_target_parallel, +Directive::OMPD_target_parallel_do, +Directive::OMPD_target_parallel_do_simd, +Directive::OMPD
[clang] e5a524b - Revert "Revert "[Flang][Sema] Move directive sets to a shared location""
Author: Sergio Afonso Date: 2023-08-10T11:54:45+01:00 New Revision: e5a524b8b5e9d11fbb12ad8ed9b2d998e05e6119 URL: https://github.com/llvm/llvm-project/commit/e5a524b8b5e9d11fbb12ad8ed9b2d998e05e6119 DIFF: https://github.com/llvm/llvm-project/commit/e5a524b8b5e9d11fbb12ad8ed9b2d998e05e6119.diff LOG: Revert "Revert "[Flang][Sema] Move directive sets to a shared location"" This reverts commit f48969f90769f37e042025dba6c544eeddd6d3e6. Differential Revision: https://reviews.llvm.org/D157493 Added: flang/include/flang/Semantics/openmp-directive-sets.h Modified: clang/docs/tools/clang-formatted-files.txt flang/lib/Semantics/check-omp-structure.cpp flang/lib/Semantics/check-omp-structure.h flang/lib/Semantics/resolve-directives.cpp Removed: diff --git a/clang/docs/tools/clang-formatted-files.txt b/clang/docs/tools/clang-formatted-files.txt index 27aab1e1305245..e22f01e8c150e1 100644 --- a/clang/docs/tools/clang-formatted-files.txt +++ b/clang/docs/tools/clang-formatted-files.txt @@ -2185,6 +2185,8 @@ flang/include/flang/Runtime/time-intrinsic.h flang/include/flang/Runtime/transformational.h flang/include/flang/Runtime/type-code.h flang/include/flang/Semantics/attr.h +flang/include/flang/Semantics/expression.h +flang/include/flang/Semantics/openmp-directive-sets.h flang/include/flang/Semantics/runtime-type-info.h flang/include/flang/Semantics/scope.h flang/include/flang/Semantics/semantics.h diff --git a/flang/include/flang/Semantics/openmp-directive-sets.h b/flang/include/flang/Semantics/openmp-directive-sets.h new file mode 100644 index 00..29bec4994a3d5b --- /dev/null +++ b/flang/include/flang/Semantics/openmp-directive-sets.h @@ -0,0 +1,283 @@ +//===-- include/flang/Semantics/openmp-directive-sets.h -*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#ifndef FORTRAN_SEMANTICS_OPENMP_DIRECTIVE_SETS_H_ +#define FORTRAN_SEMANTICS_OPENMP_DIRECTIVE_SETS_H_ + +#include "flang/Common/enum-set.h" +#include "llvm/Frontend/OpenMP/OMPConstants.h" + +using OmpDirectiveSet = Fortran::common::EnumSet; + +namespace llvm::omp { +//===--===// +// Directive sets for single directives +//===--===// +// - topSet: The directive appears alone or as the first in a +// combined construct. +// - allSet: All standalone or combined uses of the directive. + +static const OmpDirectiveSet topParallelSet{ +Directive::OMPD_parallel, +Directive::OMPD_parallel_do, +Directive::OMPD_parallel_do_simd, +Directive::OMPD_parallel_sections, +Directive::OMPD_parallel_workshare, +}; + +static const OmpDirectiveSet allParallelSet{ +Directive::OMPD_distribute_parallel_do, +Directive::OMPD_distribute_parallel_do_simd, +Directive::OMPD_parallel, +Directive::OMPD_parallel_do, +Directive::OMPD_parallel_do_simd, +Directive::OMPD_parallel_sections, +Directive::OMPD_parallel_workshare, +Directive::OMPD_target_parallel, +Directive::OMPD_target_parallel_do, +Directive::OMPD_target_parallel_do_simd, +Directive::OMPD_target_teams_distribute_parallel_do, +Directive::OMPD_target_teams_distribute_parallel_do_simd, +Directive::OMPD_teams_distribute_parallel_do, +Directive::OMPD_teams_distribute_parallel_do_simd, +}; + +static const OmpDirectiveSet topDoSet{ +Directive::OMPD_do, +Directive::OMPD_do_simd, +}; + +static const OmpDirectiveSet allDoSet{ +Directive::OMPD_distribute_parallel_do, +Directive::OMPD_distribute_parallel_do_simd, +Directive::OMPD_parallel_do, +Directive::OMPD_parallel_do_simd, +Directive::OMPD_do, +Directive::OMPD_do_simd, +Directive::OMPD_target_parallel_do, +Directive::OMPD_target_parallel_do_simd, +Directive::OMPD_target_teams_distribute_parallel_do, +Directive::OMPD_target_teams_distribute_parallel_do_simd, +Directive::OMPD_teams_distribute_parallel_do, +Directive::OMPD_teams_distribute_parallel_do_simd, +}; + +static const OmpDirectiveSet topTaskloopSet{ +Directive::OMPD_taskloop, +Directive::OMPD_taskloop_simd, +}; + +static const OmpDirectiveSet allTaskloopSet{topTaskloopSet}; + +static const OmpDirectiveSet topTargetSet{ +Directive::OMPD_target, +Directive::OMPD_target_parallel, +Directive::OMPD_target_parallel_do, +Directive::OMPD_target_parallel_do_simd, +Directive::OMPD_target_simd, +Directive::OMPD_target_teams, +Directive::OMPD_target_teams_distribute, +Directive::OMPD_target_teams_distribute_parallel_do, +Directive::OMP
[clang] 33be834 - [flang][driver][openmp] Write MLIR for -save-temps
Author: Sergio Afonso Date: 2023-03-24T17:13:40Z New Revision: 33be83415c9b5e9a874fcbddee8e64ecf464c203 URL: https://github.com/llvm/llvm-project/commit/33be83415c9b5e9a874fcbddee8e64ecf464c203 DIFF: https://github.com/llvm/llvm-project/commit/33be83415c9b5e9a874fcbddee8e64ecf464c203.diff LOG: [flang][driver][openmp] Write MLIR for -save-temps This patch adds support for producing MLIR files when using -save-temps on flang. One MLIR file will be produced before lowering and optimization passes, containing the operations produced by the PFT-to-MLIR lowering bridge, and another at the end of the process, just before LLVM IR generation. This is accomplished by forwarding the -save-temps flag from the driver to the frontend, and modifying it to output MLIR files accordingly. Differential Revision: https://reviews.llvm.org/D146075 Added: flang/test/Driver/save-mlir-temps.f90 Modified: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Flang.cpp flang/include/flang/Frontend/CodeGenOptions.h flang/lib/Frontend/CompilerInvocation.cpp flang/lib/Frontend/FrontendActions.cpp flang/test/Driver/driver-help.f90 flang/test/Driver/frontend-forwarding.f90 Removed: diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 821e86c0260f3..6eda29771cf96 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -4270,9 +4270,9 @@ def no_offload_add_rpath: Flag<["--"], "no-offload-add-rpath">, Flags<[NoArgumen Alias; def r : Flag<["-"], "r">, Flags<[LinkerInput,NoArgumentUnused]>, Group; -def save_temps_EQ : Joined<["-", "--"], "save-temps=">, Flags<[CC1Option, FlangOption, NoXarchOption]>, +def save_temps_EQ : Joined<["-", "--"], "save-temps=">, Flags<[CC1Option, FlangOption, FC1Option, NoXarchOption]>, HelpText<"Save intermediate compilation results.">; -def save_temps : Flag<["-", "--"], "save-temps">, Flags<[FlangOption, NoXarchOption]>, +def save_temps : Flag<["-", "--"], "save-temps">, Flags<[FlangOption, FC1Option, NoXarchOption]>, Alias, AliasArgs<["cwd"]>, HelpText<"Save intermediate compilation results">; def save_stats_EQ : Joined<["-", "--"], "save-stats=">, Flags<[NoXarchOption]>, diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp index 23083ff3795b5..164238d889b53 100644 --- a/clang/lib/Driver/ToolChains/Flang.cpp +++ b/clang/lib/Driver/ToolChains/Flang.cpp @@ -405,6 +405,9 @@ void Flang::ConstructJob(Compilation &C, const JobAction &JA, assert(Input.isFilename() && "Invalid input."); + if (Args.getLastArg(options::OPT_save_temps_EQ)) +Args.AddLastArg(CmdArgs, options::OPT_save_temps_EQ); + addDashXForInput(Args, Input, CmdArgs); CmdArgs.push_back(Input.getFilename()); diff --git a/flang/include/flang/Frontend/CodeGenOptions.h b/flang/include/flang/Frontend/CodeGenOptions.h index 8de150cdfb158..925de7fc319e7 100644 --- a/flang/include/flang/Frontend/CodeGenOptions.h +++ b/flang/include/flang/Frontend/CodeGenOptions.h @@ -21,6 +21,7 @@ #include "llvm/Transforms/Instrumentation/AddressSanitizerOptions.h" #include #include +#include #include #include @@ -53,6 +54,9 @@ class CodeGenOptions : public CodeGenOptionsBase { /// are offloading binaries containing device images and metadata. std::vector OffloadObjects; + /// The directory where temp files are stored if specified by -save-temps + std::optional SaveTempsDir; + // Define accessors/mutators for code generation options of enumeration type. #define CODEGENOPT(Name, Bits, Default) #define ENUM_CODEGENOPT(Name, Type, Bits, Default) \ diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp index a4183a52115b6..49dfeeddabd1b 100644 --- a/flang/lib/Frontend/CompilerInvocation.cpp +++ b/flang/lib/Frontend/CompilerInvocation.cpp @@ -150,6 +150,9 @@ static void parseCodeGenArgs(Fortran::frontend::CodeGenOptions &opts, opts.PrepareForThinLTO = true; } + if (auto *a = args.getLastArg(clang::driver::options::OPT_save_temps_EQ)) +opts.SaveTempsDir = a->getValue(); + // -mrelocation-model option. if (const llvm::opt::Arg *a = args.getLastArg(clang::driver::options::OPT_mrelocation_model)) { diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp index b723fe89387cd..9be43eb700a48 100644 --- a/flang/lib/Frontend/FrontendActions.cpp +++ b/flang/lib/Frontend/FrontendActions.cpp @@ -13,6 +13,7 @@ #include "flang/Frontend/FrontendActions.h" #include "flang/Common/default-kinds.h" #include "flang/Frontend/CompilerInstance.h" +#include "flang/Frontend/CompilerInvocation.h" #include "flang/Frontend/FrontendOptions.h" #include "flang/Frontend/PreprocessorOptions.h" #include "flang/Lower/B
[clang] 2266f4a - [Flang][Driver][OpenMP] Enable options for selecting offloading phases in flang
Author: Sergio Afonso Date: 2023-04-12T14:59:37+01:00 New Revision: 2266f4a3b38f4b59d313cdd2a9209952afb29d14 URL: https://github.com/llvm/llvm-project/commit/2266f4a3b38f4b59d313cdd2a9209952afb29d14 DIFF: https://github.com/llvm/llvm-project/commit/2266f4a3b38f4b59d313cdd2a9209952afb29d14.diff LOG: [Flang][Driver][OpenMP] Enable options for selecting offloading phases in flang This patch unlocks the "--offload-device-only", "--offload-host-only" and "--offload-host-device" options available in Clang for use by the Flang driver. These can be used to modify the behavior of the driver to select which compilation invocations are triggered during OpenMP offloading. Differential Revision: https://reviews.llvm.org/D147941 Added: flang/test/Driver/omp-driver-offload.f90 Modified: clang/include/clang/Driver/Options.td flang/test/Driver/driver-help-hidden.f90 flang/test/Driver/driver-help.f90 Removed: flang/test/Driver/omp-frontend-forwarding.f90 diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 0e3c7b708071f..7a2d9bf2a3915 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -2735,11 +2735,11 @@ def offload_new_driver : Flag<["--"], "offload-new-driver">, Flags<[CC1Option]>, MarshallingInfoFlag>, HelpText<"Use the new driver for offloading compilation.">; def no_offload_new_driver : Flag<["--"], "no-offload-new-driver">, Flags<[CC1Option]>, Group, HelpText<"Don't Use the new driver for offloading compilation.">; -def offload_device_only : Flag<["--"], "offload-device-only">, +def offload_device_only : Flag<["--"], "offload-device-only">, Flags<[FlangOption]>, HelpText<"Only compile for the offloading device.">; -def offload_host_only : Flag<["--"], "offload-host-only">, +def offload_host_only : Flag<["--"], "offload-host-only">, Flags<[FlangOption]>, HelpText<"Only compile for the offloading host.">; -def offload_host_device : Flag<["--"], "offload-host-device">, +def offload_host_device : Flag<["--"], "offload-host-device">, Flags<[FlangOption]>, HelpText<"Only compile for the offloading host.">; def cuda_device_only : Flag<["--"], "cuda-device-only">, Alias, HelpText<"Compile CUDA code for device only">; diff --git a/flang/test/Driver/driver-help-hidden.f90 b/flang/test/Driver/driver-help-hidden.f90 index 1a5fb4e996294..c365fbf16de35 100644 --- a/flang/test/Driver/driver-help-hidden.f90 +++ b/flang/test/Driver/driver-help-hidden.f90 @@ -69,6 +69,9 @@ ! CHECK-NEXT: -mmlir Additional arguments to forward to MLIR's option processing ! CHECK-NEXT: -module-dir Put MODULE files in ! CHECK-NEXT: -nocpp Disable predefined and command line preprocessor macros +! CHECK-NEXT: --offload-device-only Only compile for the offloading device. +! CHECK-NEXT: --offload-host-device Only compile for the offloading host. +! CHECK-NEXT: --offload-host-only Only compile for the offloading host. ! CHECK-NEXT: -o Write output to ! CHECK-NEXT: -pedantic Warn on language extensions ! CHECK-NEXT: -print-effective-triple Print the effective target triple diff --git a/flang/test/Driver/driver-help.f90 b/flang/test/Driver/driver-help.f90 index 84707791a2e32..25871db048911 100644 --- a/flang/test/Driver/driver-help.f90 +++ b/flang/test/Driver/driver-help.f90 @@ -65,6 +65,9 @@ ! HELP-NEXT: -mmlir Additional arguments to forward to MLIR's option processing ! HELP-NEXT: -module-dir Put MODULE files in ! HELP-NEXT: -nocpp Disable predefined and command line preprocessor macros +! HELP-NEXT: --offload-device-only Only compile for the offloading device. +! HELP-NEXT: --offload-host-device Only compile for the offloading host. +! HELP-NEXT: --offload-host-only Only compile for the offloading host. ! HELP-NEXT: -o Write output to ! HELP-NEXT: -pedantic Warn on language extensions ! HELP-NEXT: -print-effective-triple Print the effective target triple diff --git a/flang/test/Driver/omp-frontend-forwarding.f90 b/flang/test/Driver/omp-driver-offload.f90 similarity index 69% rename from flang/test/Driver/omp-frontend-forwarding.f90 rename to flang/test/Driver/omp-driver-offload.f90 index ef4875ce2fae8..ec107261d4c2b 100644 --- a/flang/test/Driver/omp-frontend-forwarding.f90 +++ b/flang/test/Driver/omp-driver-offload.f90 @@ -7,6 +7,42 @@ ! CHECK-OPENMP: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}}.f90" ! CHECK-OPENMP-NOT: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}} "-fopenmp-is-device" {{.*}}.f90" +! Test regular -fopenmp with offload, and invocation filtering options +! RUN: %flang -S -### %s -o %t 2>&1 \ +! RUN: -fopenmp --offload-arch=gfx90a --offload-arch=sm_70 \ +! RUN: --target=aarch64-unknown-linux-gnu \ +! RUN: | FileCheck %s --check-prefix=OFFLOAD-
[clang] [flang] [Flang][Driver] Introduce -fopenmp-targets offloading option (PR #100152)
https://github.com/skatrak created https://github.com/llvm/llvm-project/pull/100152 This patch modifies the flang driver to introduce the `-fopenmp-targets` option to the frontend compiler invocations corresponding to the OpenMP host device on offloading-enabled compilations. This option holds the list of offloading triples associated to the compilation and is used by clang to determine whether offloading calls should be generated for the host. >From cf26a318d3b49eb6217f29405cee9fd2c20f8e8a Mon Sep 17 00:00:00 2001 From: Sergio Afonso Date: Tue, 23 Jul 2024 16:19:55 +0100 Subject: [PATCH] [Flang][Driver] Introduce -fopenmp-targets offloading option This patch modifies the flang driver to introduce the `-fopenmp-targets` option to the frontend compiler invocations corresponding to the OpenMP host device on offloading-enabled compilations. This option holds the list of offloading triples associated to the compilation and is used by clang to determine whether offloading calls should be generated for the host. --- clang/include/clang/Driver/Options.td| 2 +- clang/lib/Driver/ToolChains/Flang.cpp| 13 + flang/test/Driver/omp-driver-offload.f90 | 17 + 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 69269cf7537b0..4ef7c81fbd9e4 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3530,7 +3530,7 @@ def fopenmp_use_tls : Flag<["-"], "fopenmp-use-tls">, Group, def fnoopenmp_use_tls : Flag<["-"], "fnoopenmp-use-tls">, Group, Flags<[NoArgumentUnused, HelpHidden]>, Visibility<[ClangOption, CC1Option]>; def fopenmp_targets_EQ : CommaJoined<["-"], "fopenmp-targets=">, - Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option, FlangOption]>, + Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>, HelpText<"Specify comma-separated list of triples OpenMP offloading targets to be supported">; def fopenmp_relocatable_target : Flag<["-"], "fopenmp-relocatable-target">, Group, Flags<[NoArgumentUnused, HelpHidden]>, diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp index c4f2375c64034..b0a3528dce05d 100644 --- a/clang/lib/Driver/ToolChains/Flang.cpp +++ b/clang/lib/Driver/ToolChains/Flang.cpp @@ -12,6 +12,7 @@ #include "clang/Basic/CodeGenOptions.h" #include "clang/Driver/Options.h" +#include "llvm/ADT/StringExtras.h" #include "llvm/Frontend/Debug/Options.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" @@ -492,6 +493,18 @@ void Flang::addOffloadOptions(Compilation &C, const InputInfoList &Inputs, if (Args.hasArg(options::OPT_nogpulib)) CmdArgs.push_back("-nogpulib"); } + + // For all the host OpenMP offloading compile jobs we need to pass the targets + // information using -fopenmp-targets= option. + if (JA.isHostOffloading(Action::OFK_OpenMP)) { +SmallString<128> Targets("-fopenmp-targets="); + +SmallVector Triples; +auto TCRange = C.getOffloadToolChains(); +std::transform(TCRange.first, TCRange.second, std::back_inserter(Triples), + [](auto TC) { return TC.second->getTripleString(); }); +CmdArgs.push_back(Args.MakeArgString(Targets + llvm::join(Triples, ","))); + } } static void addFloatingPointOptions(const Driver &D, const ArgList &Args, diff --git a/flang/test/Driver/omp-driver-offload.f90 b/flang/test/Driver/omp-driver-offload.f90 index 6fb4f4ca1..70fb8c7298e77 100644 --- a/flang/test/Driver/omp-driver-offload.f90 +++ b/flang/test/Driver/omp-driver-offload.f90 @@ -227,3 +227,20 @@ ! FORCE-USM-OFFLOAD-SAME: "-fopenmp" "-fopenmp-force-usm" ! FORCE-USM-OFFLOAD-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" ! FORCE-USM-OFFLOAD-SAME: "-fopenmp" "-fopenmp-force-usm" + +! RUN: %flang -S -### %s -o %t 2>&1 \ +! RUN: -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa \ +! RUN: --target=aarch64-unknown-linux-gnu \ +! RUN: | FileCheck %s --check-prefix=OFFLOAD-TARGETS + +! RUN: %flang -S -### %s -o %t 2>&1 \ +! RUN: -fopenmp --offload-arch=gfx90a \ +! RUN: --target=aarch64-unknown-linux-gnu \ +! RUN: | FileCheck %s --check-prefix=OFFLOAD-TARGETS + +! OFFLOAD-TARGETS: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" +! OFFLOAD-TARGETS-SAME: "-fopenmp-targets=amdgcn-amd-amdhsa" +! OFFLOAD-TARGETS-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" +! OFFLOAD-TARGETS-NOT: -fopenmp-targets +! OFFLOAD-TARGETS: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" +! OFFLOAD-TARGETS-SAME: "-fopenmp-targets=amdgcn-amd-amdhsa" ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Introduce -fopenmp-targets offloading option (PR #100152)
skatrak wrote: > Code changes look good to me. I can't speak to the needs of anyone else using > offloading. > > I see this is copied exactly from `Toolchains/Clang.cpp`. I think that's okay > for such a small bit of code. Thank you for the quick review. Yes, it's basically copied from clang, just like most of the other code we have in there, hopefully we can at some point refactor things a bit more. This is part of a PR stack, and the last one shows a use case for this: - https://github.com/llvm/llvm-project/pull/100154 - https://github.com/llvm/llvm-project/pull/100155 - https://github.com/llvm/llvm-project/pull/100156 https://github.com/llvm/llvm-project/pull/100152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Introduce -fopenmp-targets offloading option (PR #100152)
https://github.com/skatrak updated https://github.com/llvm/llvm-project/pull/100152 >From cf26a318d3b49eb6217f29405cee9fd2c20f8e8a Mon Sep 17 00:00:00 2001 From: Sergio Afonso Date: Tue, 23 Jul 2024 16:19:55 +0100 Subject: [PATCH 1/2] [Flang][Driver] Introduce -fopenmp-targets offloading option This patch modifies the flang driver to introduce the `-fopenmp-targets` option to the frontend compiler invocations corresponding to the OpenMP host device on offloading-enabled compilations. This option holds the list of offloading triples associated to the compilation and is used by clang to determine whether offloading calls should be generated for the host. --- clang/include/clang/Driver/Options.td| 2 +- clang/lib/Driver/ToolChains/Flang.cpp| 13 + flang/test/Driver/omp-driver-offload.f90 | 17 + 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 69269cf7537b0..4ef7c81fbd9e4 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3530,7 +3530,7 @@ def fopenmp_use_tls : Flag<["-"], "fopenmp-use-tls">, Group, def fnoopenmp_use_tls : Flag<["-"], "fnoopenmp-use-tls">, Group, Flags<[NoArgumentUnused, HelpHidden]>, Visibility<[ClangOption, CC1Option]>; def fopenmp_targets_EQ : CommaJoined<["-"], "fopenmp-targets=">, - Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option, FlangOption]>, + Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>, HelpText<"Specify comma-separated list of triples OpenMP offloading targets to be supported">; def fopenmp_relocatable_target : Flag<["-"], "fopenmp-relocatable-target">, Group, Flags<[NoArgumentUnused, HelpHidden]>, diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp index c4f2375c64034..b0a3528dce05d 100644 --- a/clang/lib/Driver/ToolChains/Flang.cpp +++ b/clang/lib/Driver/ToolChains/Flang.cpp @@ -12,6 +12,7 @@ #include "clang/Basic/CodeGenOptions.h" #include "clang/Driver/Options.h" +#include "llvm/ADT/StringExtras.h" #include "llvm/Frontend/Debug/Options.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" @@ -492,6 +493,18 @@ void Flang::addOffloadOptions(Compilation &C, const InputInfoList &Inputs, if (Args.hasArg(options::OPT_nogpulib)) CmdArgs.push_back("-nogpulib"); } + + // For all the host OpenMP offloading compile jobs we need to pass the targets + // information using -fopenmp-targets= option. + if (JA.isHostOffloading(Action::OFK_OpenMP)) { +SmallString<128> Targets("-fopenmp-targets="); + +SmallVector Triples; +auto TCRange = C.getOffloadToolChains(); +std::transform(TCRange.first, TCRange.second, std::back_inserter(Triples), + [](auto TC) { return TC.second->getTripleString(); }); +CmdArgs.push_back(Args.MakeArgString(Targets + llvm::join(Triples, ","))); + } } static void addFloatingPointOptions(const Driver &D, const ArgList &Args, diff --git a/flang/test/Driver/omp-driver-offload.f90 b/flang/test/Driver/omp-driver-offload.f90 index 6fb4f4ca1..70fb8c7298e77 100644 --- a/flang/test/Driver/omp-driver-offload.f90 +++ b/flang/test/Driver/omp-driver-offload.f90 @@ -227,3 +227,20 @@ ! FORCE-USM-OFFLOAD-SAME: "-fopenmp" "-fopenmp-force-usm" ! FORCE-USM-OFFLOAD-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" ! FORCE-USM-OFFLOAD-SAME: "-fopenmp" "-fopenmp-force-usm" + +! RUN: %flang -S -### %s -o %t 2>&1 \ +! RUN: -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa \ +! RUN: --target=aarch64-unknown-linux-gnu \ +! RUN: | FileCheck %s --check-prefix=OFFLOAD-TARGETS + +! RUN: %flang -S -### %s -o %t 2>&1 \ +! RUN: -fopenmp --offload-arch=gfx90a \ +! RUN: --target=aarch64-unknown-linux-gnu \ +! RUN: | FileCheck %s --check-prefix=OFFLOAD-TARGETS + +! OFFLOAD-TARGETS: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" +! OFFLOAD-TARGETS-SAME: "-fopenmp-targets=amdgcn-amd-amdhsa" +! OFFLOAD-TARGETS-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" +! OFFLOAD-TARGETS-NOT: -fopenmp-targets +! OFFLOAD-TARGETS: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" +! OFFLOAD-TARGETS-SAME: "-fopenmp-targets=amdgcn-amd-amdhsa" >From 848937e7aad4440a7674fd91bc5871bbcc856f91 Mon Sep 17 00:00:00 2001 From: Sergio Afonso Date: Wed, 24 Jul 2024 11:29:19 +0100 Subject: [PATCH 2/2] Attempt to fix buildbot issue --- flang/test/Driver/omp-driver-offload.f90 | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/flang/test/Driver/omp-driver-offload.f90 b/flang/test/Driver/omp-driver-offload.f90 index 70fb8c7298e77..b8e1e4c3951fe 100644 --- a/flang/test/Driver/omp-driver-offload.f90 +++ b/flang/test/Driver/omp-driver-offload.f90 @@ -230,17 +230,16 @@ ! RUN: %flang -S -### %s -o %t 2>&1 \ ! RUN: -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa \ -!
[clang] [flang] [Flang][Driver] Introduce -fopenmp-targets offloading option (PR #100152)
@@ -492,6 +493,18 @@ void Flang::addOffloadOptions(Compilation &C, const InputInfoList &Inputs, if (Args.hasArg(options::OPT_nogpulib)) CmdArgs.push_back("-nogpulib"); } + + // For all the host OpenMP offloading compile jobs we need to pass the targets + // information using -fopenmp-targets= option. + if (JA.isHostOffloading(Action::OFK_OpenMP)) { +SmallString<128> Targets("-fopenmp-targets="); skatrak wrote: This block of code is copied from clang: https://github.com/llvm/llvm-project/blob/59e1c6cd63eb9287df6516f9a5ae564075e9c218/clang/lib/Driver/ToolChains/Clang.cpp#L7746 Would it be better to leave it unchanged, let both implementations diverge or apply these suggestions to clang too? I think it makes sense to do them, just trying to avoid divergence on duplicated code. https://github.com/llvm/llvm-project/pull/100152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Introduce -fopenmp-targets offloading option (PR #100152)
@@ -227,3 +227,20 @@ ! FORCE-USM-OFFLOAD-SAME: "-fopenmp" "-fopenmp-force-usm" ! FORCE-USM-OFFLOAD-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" ! FORCE-USM-OFFLOAD-SAME: "-fopenmp" "-fopenmp-force-usm" + skatrak wrote: Done. https://github.com/llvm/llvm-project/pull/100152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Introduce -fopenmp-targets offloading option (PR #100152)
https://github.com/skatrak updated https://github.com/llvm/llvm-project/pull/100152 >From cf26a318d3b49eb6217f29405cee9fd2c20f8e8a Mon Sep 17 00:00:00 2001 From: Sergio Afonso Date: Tue, 23 Jul 2024 16:19:55 +0100 Subject: [PATCH 1/3] [Flang][Driver] Introduce -fopenmp-targets offloading option This patch modifies the flang driver to introduce the `-fopenmp-targets` option to the frontend compiler invocations corresponding to the OpenMP host device on offloading-enabled compilations. This option holds the list of offloading triples associated to the compilation and is used by clang to determine whether offloading calls should be generated for the host. --- clang/include/clang/Driver/Options.td| 2 +- clang/lib/Driver/ToolChains/Flang.cpp| 13 + flang/test/Driver/omp-driver-offload.f90 | 17 + 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 69269cf7537b0..4ef7c81fbd9e4 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3530,7 +3530,7 @@ def fopenmp_use_tls : Flag<["-"], "fopenmp-use-tls">, Group, def fnoopenmp_use_tls : Flag<["-"], "fnoopenmp-use-tls">, Group, Flags<[NoArgumentUnused, HelpHidden]>, Visibility<[ClangOption, CC1Option]>; def fopenmp_targets_EQ : CommaJoined<["-"], "fopenmp-targets=">, - Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option, FlangOption]>, + Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>, HelpText<"Specify comma-separated list of triples OpenMP offloading targets to be supported">; def fopenmp_relocatable_target : Flag<["-"], "fopenmp-relocatable-target">, Group, Flags<[NoArgumentUnused, HelpHidden]>, diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp index c4f2375c64034..b0a3528dce05d 100644 --- a/clang/lib/Driver/ToolChains/Flang.cpp +++ b/clang/lib/Driver/ToolChains/Flang.cpp @@ -12,6 +12,7 @@ #include "clang/Basic/CodeGenOptions.h" #include "clang/Driver/Options.h" +#include "llvm/ADT/StringExtras.h" #include "llvm/Frontend/Debug/Options.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" @@ -492,6 +493,18 @@ void Flang::addOffloadOptions(Compilation &C, const InputInfoList &Inputs, if (Args.hasArg(options::OPT_nogpulib)) CmdArgs.push_back("-nogpulib"); } + + // For all the host OpenMP offloading compile jobs we need to pass the targets + // information using -fopenmp-targets= option. + if (JA.isHostOffloading(Action::OFK_OpenMP)) { +SmallString<128> Targets("-fopenmp-targets="); + +SmallVector Triples; +auto TCRange = C.getOffloadToolChains(); +std::transform(TCRange.first, TCRange.second, std::back_inserter(Triples), + [](auto TC) { return TC.second->getTripleString(); }); +CmdArgs.push_back(Args.MakeArgString(Targets + llvm::join(Triples, ","))); + } } static void addFloatingPointOptions(const Driver &D, const ArgList &Args, diff --git a/flang/test/Driver/omp-driver-offload.f90 b/flang/test/Driver/omp-driver-offload.f90 index 6fb4f4ca1..70fb8c7298e77 100644 --- a/flang/test/Driver/omp-driver-offload.f90 +++ b/flang/test/Driver/omp-driver-offload.f90 @@ -227,3 +227,20 @@ ! FORCE-USM-OFFLOAD-SAME: "-fopenmp" "-fopenmp-force-usm" ! FORCE-USM-OFFLOAD-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" ! FORCE-USM-OFFLOAD-SAME: "-fopenmp" "-fopenmp-force-usm" + +! RUN: %flang -S -### %s -o %t 2>&1 \ +! RUN: -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa \ +! RUN: --target=aarch64-unknown-linux-gnu \ +! RUN: | FileCheck %s --check-prefix=OFFLOAD-TARGETS + +! RUN: %flang -S -### %s -o %t 2>&1 \ +! RUN: -fopenmp --offload-arch=gfx90a \ +! RUN: --target=aarch64-unknown-linux-gnu \ +! RUN: | FileCheck %s --check-prefix=OFFLOAD-TARGETS + +! OFFLOAD-TARGETS: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" +! OFFLOAD-TARGETS-SAME: "-fopenmp-targets=amdgcn-amd-amdhsa" +! OFFLOAD-TARGETS-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" +! OFFLOAD-TARGETS-NOT: -fopenmp-targets +! OFFLOAD-TARGETS: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" +! OFFLOAD-TARGETS-SAME: "-fopenmp-targets=amdgcn-amd-amdhsa" >From a5d5c988044c3182fb51662f486e1b79820e29f8 Mon Sep 17 00:00:00 2001 From: Sergio Afonso Date: Wed, 24 Jul 2024 11:29:19 +0100 Subject: [PATCH 2/3] Attempt to fix buildbot issue --- .../test/Driver/omp-driver-offload-amdgpu.f90 | 18 ++ flang/test/Driver/omp-driver-offload.f90 | 11 +++ 2 files changed, 21 insertions(+), 8 deletions(-) create mode 100644 flang/test/Driver/omp-driver-offload-amdgpu.f90 diff --git a/flang/test/Driver/omp-driver-offload-amdgpu.f90 b/flang/test/Driver/omp-driver-offload-amdgpu.f90 new file mode 100644 index 0..cf806d672f396 --- /dev/null +++ b/flang/test/Dri
[clang] [flang] [Flang][Driver] Introduce -fopenmp-targets offloading option (PR #100152)
https://github.com/skatrak updated https://github.com/llvm/llvm-project/pull/100152 >From cf26a318d3b49eb6217f29405cee9fd2c20f8e8a Mon Sep 17 00:00:00 2001 From: Sergio Afonso Date: Tue, 23 Jul 2024 16:19:55 +0100 Subject: [PATCH 1/4] [Flang][Driver] Introduce -fopenmp-targets offloading option This patch modifies the flang driver to introduce the `-fopenmp-targets` option to the frontend compiler invocations corresponding to the OpenMP host device on offloading-enabled compilations. This option holds the list of offloading triples associated to the compilation and is used by clang to determine whether offloading calls should be generated for the host. --- clang/include/clang/Driver/Options.td| 2 +- clang/lib/Driver/ToolChains/Flang.cpp| 13 + flang/test/Driver/omp-driver-offload.f90 | 17 + 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 69269cf7537b0..4ef7c81fbd9e4 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3530,7 +3530,7 @@ def fopenmp_use_tls : Flag<["-"], "fopenmp-use-tls">, Group, def fnoopenmp_use_tls : Flag<["-"], "fnoopenmp-use-tls">, Group, Flags<[NoArgumentUnused, HelpHidden]>, Visibility<[ClangOption, CC1Option]>; def fopenmp_targets_EQ : CommaJoined<["-"], "fopenmp-targets=">, - Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option, FlangOption]>, + Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>, HelpText<"Specify comma-separated list of triples OpenMP offloading targets to be supported">; def fopenmp_relocatable_target : Flag<["-"], "fopenmp-relocatable-target">, Group, Flags<[NoArgumentUnused, HelpHidden]>, diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp index c4f2375c64034..b0a3528dce05d 100644 --- a/clang/lib/Driver/ToolChains/Flang.cpp +++ b/clang/lib/Driver/ToolChains/Flang.cpp @@ -12,6 +12,7 @@ #include "clang/Basic/CodeGenOptions.h" #include "clang/Driver/Options.h" +#include "llvm/ADT/StringExtras.h" #include "llvm/Frontend/Debug/Options.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" @@ -492,6 +493,18 @@ void Flang::addOffloadOptions(Compilation &C, const InputInfoList &Inputs, if (Args.hasArg(options::OPT_nogpulib)) CmdArgs.push_back("-nogpulib"); } + + // For all the host OpenMP offloading compile jobs we need to pass the targets + // information using -fopenmp-targets= option. + if (JA.isHostOffloading(Action::OFK_OpenMP)) { +SmallString<128> Targets("-fopenmp-targets="); + +SmallVector Triples; +auto TCRange = C.getOffloadToolChains(); +std::transform(TCRange.first, TCRange.second, std::back_inserter(Triples), + [](auto TC) { return TC.second->getTripleString(); }); +CmdArgs.push_back(Args.MakeArgString(Targets + llvm::join(Triples, ","))); + } } static void addFloatingPointOptions(const Driver &D, const ArgList &Args, diff --git a/flang/test/Driver/omp-driver-offload.f90 b/flang/test/Driver/omp-driver-offload.f90 index 6fb4f4ca1..70fb8c7298e77 100644 --- a/flang/test/Driver/omp-driver-offload.f90 +++ b/flang/test/Driver/omp-driver-offload.f90 @@ -227,3 +227,20 @@ ! FORCE-USM-OFFLOAD-SAME: "-fopenmp" "-fopenmp-force-usm" ! FORCE-USM-OFFLOAD-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" ! FORCE-USM-OFFLOAD-SAME: "-fopenmp" "-fopenmp-force-usm" + +! RUN: %flang -S -### %s -o %t 2>&1 \ +! RUN: -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa \ +! RUN: --target=aarch64-unknown-linux-gnu \ +! RUN: | FileCheck %s --check-prefix=OFFLOAD-TARGETS + +! RUN: %flang -S -### %s -o %t 2>&1 \ +! RUN: -fopenmp --offload-arch=gfx90a \ +! RUN: --target=aarch64-unknown-linux-gnu \ +! RUN: | FileCheck %s --check-prefix=OFFLOAD-TARGETS + +! OFFLOAD-TARGETS: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" +! OFFLOAD-TARGETS-SAME: "-fopenmp-targets=amdgcn-amd-amdhsa" +! OFFLOAD-TARGETS-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" +! OFFLOAD-TARGETS-NOT: -fopenmp-targets +! OFFLOAD-TARGETS: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" +! OFFLOAD-TARGETS-SAME: "-fopenmp-targets=amdgcn-amd-amdhsa" >From a5d5c988044c3182fb51662f486e1b79820e29f8 Mon Sep 17 00:00:00 2001 From: Sergio Afonso Date: Wed, 24 Jul 2024 11:29:19 +0100 Subject: [PATCH 2/4] Attempt to fix buildbot issue --- .../test/Driver/omp-driver-offload-amdgpu.f90 | 18 ++ flang/test/Driver/omp-driver-offload.f90 | 11 +++ 2 files changed, 21 insertions(+), 8 deletions(-) create mode 100644 flang/test/Driver/omp-driver-offload-amdgpu.f90 diff --git a/flang/test/Driver/omp-driver-offload-amdgpu.f90 b/flang/test/Driver/omp-driver-offload-amdgpu.f90 new file mode 100644 index 0..cf806d672f396 --- /dev/null +++ b/flang/test/Dri
[clang] [flang] [Flang][Driver] Introduce -fopenmp-targets offloading option (PR #100152)
skatrak wrote: I'm not very knowledgeable on the set of offloading-related options clang supports, how those impact the set of invocations and options produced by the driver, or how these options eventually impact passes and codegen. Much less about potential shortcomings and improvements to the current approach, which appears to be what the discussion here is moving to. However, I can give a brief overview of what this PR stack is about. The flang-new driver is very much based on clang, only limited in the set of options it can currently handle and adding Fortran-specific ones. In terms of offloading, it also produces a separate compiler invocation for the host and the target device, based on the same machinery and driver options as far as I can tell. One current limitation we have noticed is that the host compilation for Flang will generate offloading calls (`__tgt_target_kernel`) even when no offloading targets were specified (e.g. `--offload-arch` or `-fopenmp-targets`), resulting in linker errors: #100209. Looking into replicating what clang does to avoid this problem, I noticed that the `-fopenmp-targets` option was passed to host compiler invocations, so that it would know which offload targets are associated with it. When this is not passed, then it knows not to generate offloading calls and run directly in the host instead. This PR makes sure that it is also the case for flang-new that the `-fopenmp-targets` option is added by the driver to host offloading invocations and that `flang-new -fc1` won't complain when it gets that option. Other PRs in the stack then take this information, store it in the MLIR module and take it into consideration while generating LLVM IR for OpenMP target regions. In my opinion, the decision to potentially rename or move to a different set of offloading flags should be independent from this work, as it doesn't really add anything new, it only replicates one of the features clang already has. If a change to that is agreed upon, we can then update both clang and flang separately. If people agree, I can merge these changes and perhaps the more general discussion could be continued in discourse. https://github.com/llvm/llvm-project/pull/100152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Introduce -fopenmp-targets offloading option (PR #100152)
@@ -492,6 +493,18 @@ void Flang::addOffloadOptions(Compilation &C, const InputInfoList &Inputs, if (Args.hasArg(options::OPT_nogpulib)) CmdArgs.push_back("-nogpulib"); } + + // For all the host OpenMP offloading compile jobs we need to pass the targets + // information using -fopenmp-targets= option. + if (JA.isHostOffloading(Action::OFK_OpenMP)) { +SmallString<128> Targets("-fopenmp-targets="); skatrak wrote: > For now, I would grudgingly accept the code as-is if this was done for the > rest of the file, but hope that we can avoid all this code duplication in the > future. > > Did you consider to just extract the lines from `Clang.cpp` into another > function to some location this is accessible to both? I agree that we should try to avoid duplicating code like this as much as possible. In this case, I just followed what's been done for other similar options and just copied it from clang. After a quick look, I don't see an obvious place to add this shared code, but it looks like one way to get there would be to create a `Driver` or similar class deriving from `Tool` and make that the shared parent for the `Clang` and `Flang` classes, putting in it common option processing code used by both. Something like this would probably be best done on its own patch after some discussion, though. In any case, I think @banach-space is the expert on this area, so maybe he knows of any plans already in place to improve this situation or can give us some pointers on how to better deal with this in the short term. https://github.com/llvm/llvm-project/pull/100152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Introduce -fopenmp-targets offloading option (PR #100152)
skatrak wrote: > The only issue I see is that this PR adds a new driver flag that may be used > to compile the applications. Renaming the user visible flags is not that > easy. But if they get renamed before any compiler release then it should be > fine. Thank you for the comment. Actually, this PR doesn't add any new driver options to flang. It just enables the `-fopenmp-targets` option to be forwarded to the flang frontend (`flang-new -fc1`) as well. Previously, it was only accepted by the driver. https://github.com/llvm/llvm-project/pull/100152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Introduce -fopenmp-targets offloading option (PR #100152)
https://github.com/skatrak updated https://github.com/llvm/llvm-project/pull/100152 >From cf26a318d3b49eb6217f29405cee9fd2c20f8e8a Mon Sep 17 00:00:00 2001 From: Sergio Afonso Date: Tue, 23 Jul 2024 16:19:55 +0100 Subject: [PATCH 1/5] [Flang][Driver] Introduce -fopenmp-targets offloading option This patch modifies the flang driver to introduce the `-fopenmp-targets` option to the frontend compiler invocations corresponding to the OpenMP host device on offloading-enabled compilations. This option holds the list of offloading triples associated to the compilation and is used by clang to determine whether offloading calls should be generated for the host. --- clang/include/clang/Driver/Options.td| 2 +- clang/lib/Driver/ToolChains/Flang.cpp| 13 + flang/test/Driver/omp-driver-offload.f90 | 17 + 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 69269cf7537b0..4ef7c81fbd9e4 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3530,7 +3530,7 @@ def fopenmp_use_tls : Flag<["-"], "fopenmp-use-tls">, Group, def fnoopenmp_use_tls : Flag<["-"], "fnoopenmp-use-tls">, Group, Flags<[NoArgumentUnused, HelpHidden]>, Visibility<[ClangOption, CC1Option]>; def fopenmp_targets_EQ : CommaJoined<["-"], "fopenmp-targets=">, - Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option, FlangOption]>, + Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>, HelpText<"Specify comma-separated list of triples OpenMP offloading targets to be supported">; def fopenmp_relocatable_target : Flag<["-"], "fopenmp-relocatable-target">, Group, Flags<[NoArgumentUnused, HelpHidden]>, diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp index c4f2375c64034..b0a3528dce05d 100644 --- a/clang/lib/Driver/ToolChains/Flang.cpp +++ b/clang/lib/Driver/ToolChains/Flang.cpp @@ -12,6 +12,7 @@ #include "clang/Basic/CodeGenOptions.h" #include "clang/Driver/Options.h" +#include "llvm/ADT/StringExtras.h" #include "llvm/Frontend/Debug/Options.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" @@ -492,6 +493,18 @@ void Flang::addOffloadOptions(Compilation &C, const InputInfoList &Inputs, if (Args.hasArg(options::OPT_nogpulib)) CmdArgs.push_back("-nogpulib"); } + + // For all the host OpenMP offloading compile jobs we need to pass the targets + // information using -fopenmp-targets= option. + if (JA.isHostOffloading(Action::OFK_OpenMP)) { +SmallString<128> Targets("-fopenmp-targets="); + +SmallVector Triples; +auto TCRange = C.getOffloadToolChains(); +std::transform(TCRange.first, TCRange.second, std::back_inserter(Triples), + [](auto TC) { return TC.second->getTripleString(); }); +CmdArgs.push_back(Args.MakeArgString(Targets + llvm::join(Triples, ","))); + } } static void addFloatingPointOptions(const Driver &D, const ArgList &Args, diff --git a/flang/test/Driver/omp-driver-offload.f90 b/flang/test/Driver/omp-driver-offload.f90 index 6fb4f4ca1..70fb8c7298e77 100644 --- a/flang/test/Driver/omp-driver-offload.f90 +++ b/flang/test/Driver/omp-driver-offload.f90 @@ -227,3 +227,20 @@ ! FORCE-USM-OFFLOAD-SAME: "-fopenmp" "-fopenmp-force-usm" ! FORCE-USM-OFFLOAD-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" ! FORCE-USM-OFFLOAD-SAME: "-fopenmp" "-fopenmp-force-usm" + +! RUN: %flang -S -### %s -o %t 2>&1 \ +! RUN: -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa \ +! RUN: --target=aarch64-unknown-linux-gnu \ +! RUN: | FileCheck %s --check-prefix=OFFLOAD-TARGETS + +! RUN: %flang -S -### %s -o %t 2>&1 \ +! RUN: -fopenmp --offload-arch=gfx90a \ +! RUN: --target=aarch64-unknown-linux-gnu \ +! RUN: | FileCheck %s --check-prefix=OFFLOAD-TARGETS + +! OFFLOAD-TARGETS: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" +! OFFLOAD-TARGETS-SAME: "-fopenmp-targets=amdgcn-amd-amdhsa" +! OFFLOAD-TARGETS-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" +! OFFLOAD-TARGETS-NOT: -fopenmp-targets +! OFFLOAD-TARGETS: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" +! OFFLOAD-TARGETS-SAME: "-fopenmp-targets=amdgcn-amd-amdhsa" >From a5d5c988044c3182fb51662f486e1b79820e29f8 Mon Sep 17 00:00:00 2001 From: Sergio Afonso Date: Wed, 24 Jul 2024 11:29:19 +0100 Subject: [PATCH 2/5] Attempt to fix buildbot issue --- .../test/Driver/omp-driver-offload-amdgpu.f90 | 18 ++ flang/test/Driver/omp-driver-offload.f90 | 11 +++ 2 files changed, 21 insertions(+), 8 deletions(-) create mode 100644 flang/test/Driver/omp-driver-offload-amdgpu.f90 diff --git a/flang/test/Driver/omp-driver-offload-amdgpu.f90 b/flang/test/Driver/omp-driver-offload-amdgpu.f90 new file mode 100644 index 0..cf806d672f396 --- /dev/null +++ b/flang/test/Dri
[clang] [flang] [Flang][Driver] Introduce -fopenmp-targets offloading option (PR #100152)
@@ -492,6 +493,18 @@ void Flang::addOffloadOptions(Compilation &C, const InputInfoList &Inputs, if (Args.hasArg(options::OPT_nogpulib)) CmdArgs.push_back("-nogpulib"); } + + // For all the host OpenMP offloading compile jobs we need to pass the targets + // information using -fopenmp-targets= option. + if (JA.isHostOffloading(Action::OFK_OpenMP)) { +SmallString<128> Targets("-fopenmp-targets="); skatrak wrote: Thank you for the pointer, @banach-space. I moved this into a new function in CommonArgs, so there's no longer code duplication added for this. Addressed @Meinersbur comments as well. https://github.com/llvm/llvm-project/pull/100152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Introduce -fopenmp-targets offloading option (PR #100152)
@@ -492,6 +493,18 @@ void Flang::addOffloadOptions(Compilation &C, const InputInfoList &Inputs, if (Args.hasArg(options::OPT_nogpulib)) CmdArgs.push_back("-nogpulib"); } + + // For all the host OpenMP offloading compile jobs we need to pass the targets + // information using -fopenmp-targets= option. + if (JA.isHostOffloading(Action::OFK_OpenMP)) { +SmallString<128> Targets("-fopenmp-targets="); + +SmallVector Triples; +auto TCRange = C.getOffloadToolChains(); +std::transform(TCRange.first, TCRange.second, std::back_inserter(Triples), + [](auto TC) { return TC.second->getTripleString(); }); +CmdArgs.push_back(Args.MakeArgString(Targets + llvm::join(Triples, ","))); skatrak wrote: Done. https://github.com/llvm/llvm-project/pull/100152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Introduce -fopenmp-targets offloading option (PR #100152)
@@ -492,6 +493,18 @@ void Flang::addOffloadOptions(Compilation &C, const InputInfoList &Inputs, if (Args.hasArg(options::OPT_nogpulib)) CmdArgs.push_back("-nogpulib"); } + + // For all the host OpenMP offloading compile jobs we need to pass the targets + // information using -fopenmp-targets= option. + if (JA.isHostOffloading(Action::OFK_OpenMP)) { +SmallString<128> Targets("-fopenmp-targets="); + +SmallVector Triples; skatrak wrote: Done. https://github.com/llvm/llvm-project/pull/100152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Introduce -fopenmp-targets offloading option (PR #100152)
https://github.com/skatrak updated https://github.com/llvm/llvm-project/pull/100152 >From 3861b28deeb1b558f182f2ab5680b123fd94c005 Mon Sep 17 00:00:00 2001 From: Sergio Afonso Date: Tue, 23 Jul 2024 16:19:55 +0100 Subject: [PATCH] [Flang][Driver] Introduce -fopenmp-targets offloading option This patch modifies the flang driver to introduce the `-fopenmp-targets` option to the frontend compiler invocations corresponding to the OpenMP host device on offloading-enabled compilations. This option holds the list of offloading triples associated to the compilation and is used by clang to determine whether offloading calls should be generated for the host. --- clang/include/clang/Driver/Options.td | 2 +- clang/lib/Driver/ToolChains/Clang.cpp | 12 +--- clang/lib/Driver/ToolChains/CommonArgs.cpp | 19 +++ clang/lib/Driver/ToolChains/CommonArgs.h | 5 + clang/lib/Driver/ToolChains/Flang.cpp | 3 +++ flang/test/Driver/omp-driver-offload.f90 | 18 ++ 6 files changed, 47 insertions(+), 12 deletions(-) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index c8c56dbb51b28..59c6bb70d75a2 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3532,7 +3532,7 @@ def fopenmp_use_tls : Flag<["-"], "fopenmp-use-tls">, Group, def fnoopenmp_use_tls : Flag<["-"], "fnoopenmp-use-tls">, Group, Flags<[NoArgumentUnused, HelpHidden]>, Visibility<[ClangOption, CC1Option]>; def fopenmp_targets_EQ : CommaJoined<["-"], "fopenmp-targets=">, - Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option, FlangOption]>, + Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>, HelpText<"Specify comma-separated list of triples OpenMP offloading targets to be supported">; def fopenmp_relocatable_target : Flag<["-"], "fopenmp-relocatable-target">, Group, Flags<[NoArgumentUnused, HelpHidden]>, diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 843d68c85bc59..0f1141ed8bcd9 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -7789,17 +7789,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, options::OPT_mno_amdgpu_ieee); } - // For all the host OpenMP offloading compile jobs we need to pass the targets - // information using -fopenmp-targets= option. - if (JA.isHostOffloading(Action::OFK_OpenMP)) { -SmallString<128> Targets("-fopenmp-targets="); - -SmallVector Triples; -auto TCRange = C.getOffloadToolChains(); -std::transform(TCRange.first, TCRange.second, std::back_inserter(Triples), - [](auto TC) { return TC.second->getTripleString(); }); -CmdArgs.push_back(Args.MakeArgString(Targets + llvm::join(Triples, ","))); - } + addOpenMPHostOffloadingArgs(C, JA, Args, CmdArgs); bool VirtualFunctionElimination = Args.hasFlag(options::OPT_fvirtual_function_elimination, diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 1e37d9d348818..487cc8345d7e3 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1244,6 +1244,25 @@ bool tools::addOpenMPRuntime(const Compilation &C, ArgStringList &CmdArgs, return true; } +void tools::addOpenMPHostOffloadingArgs(const Compilation &C, +const JobAction &JA, +const llvm::opt::ArgList &Args, +llvm::opt::ArgStringList &CmdArgs) { + if (!JA.isHostOffloading(Action::OFK_OpenMP)) +return; + + // For all the host OpenMP offloading compile jobs we need to pass the targets + // information using -fopenmp-targets= option. + constexpr llvm::StringLiteral Targets("-fopenmp-targets="); + + SmallVector Triples; + auto TCRange = C.getOffloadToolChains(); + std::transform(TCRange.first, TCRange.second, std::back_inserter(Triples), + [](auto TC) { return TC.second->getTripleString(); }); + CmdArgs.push_back( + Args.MakeArgString(Twine(Targets) + llvm::join(Triples, ","))); +} + /// Add Fortran runtime libs void tools::addFortranRuntimeLibs(const ToolChain &TC, const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { diff --git a/clang/lib/Driver/ToolChains/CommonArgs.h b/clang/lib/Driver/ToolChains/CommonArgs.h index 0d3b4d6b783ba..0c97398dfcfa3 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.h +++ b/clang/lib/Driver/ToolChains/CommonArgs.h @@ -119,6 +119,11 @@ bool addOpenMPRuntime(const Compilation &C, llvm::opt::ArgStringList &CmdArgs, bool ForceStaticHostRuntime = false, bool IsOffloadingHost = false, bool GompNeedsRT = false); +/// Adds offloading options for OpenMP host compilation to
[clang] [flang] [Flang][Driver] Introduce -fopenmp-targets offloading option (PR #100152)
skatrak wrote: Thank you all for your reviews. I think it should be ok to merge at this point, but I'll wait until tomorrow to give some time in case there are any remaining concerns about this change. https://github.com/llvm/llvm-project/pull/100152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Introduce -fopenmp-targets offloading option (PR #100152)
https://github.com/skatrak updated https://github.com/llvm/llvm-project/pull/100152 >From e59c38db58ba5ca2eef66d3b3477d5ad81043228 Mon Sep 17 00:00:00 2001 From: Sergio Afonso Date: Tue, 23 Jul 2024 16:19:55 +0100 Subject: [PATCH] [Flang][Driver] Introduce -fopenmp-targets offloading option This patch modifies the flang driver to introduce the `-fopenmp-targets` option to the frontend compiler invocations corresponding to the OpenMP host device on offloading-enabled compilations. This option holds the list of offloading triples associated to the compilation and is used by clang to determine whether offloading calls should be generated for the host. --- clang/include/clang/Driver/Options.td | 2 +- clang/lib/Driver/ToolChains/Clang.cpp | 12 +--- clang/lib/Driver/ToolChains/CommonArgs.cpp | 19 +++ clang/lib/Driver/ToolChains/CommonArgs.h | 5 + clang/lib/Driver/ToolChains/Flang.cpp | 2 ++ flang/test/Driver/omp-driver-offload.f90 | 18 ++ 6 files changed, 46 insertions(+), 12 deletions(-) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index c8c56dbb51b28..59c6bb70d75a2 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3532,7 +3532,7 @@ def fopenmp_use_tls : Flag<["-"], "fopenmp-use-tls">, Group, def fnoopenmp_use_tls : Flag<["-"], "fnoopenmp-use-tls">, Group, Flags<[NoArgumentUnused, HelpHidden]>, Visibility<[ClangOption, CC1Option]>; def fopenmp_targets_EQ : CommaJoined<["-"], "fopenmp-targets=">, - Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option, FlangOption]>, + Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>, HelpText<"Specify comma-separated list of triples OpenMP offloading targets to be supported">; def fopenmp_relocatable_target : Flag<["-"], "fopenmp-relocatable-target">, Group, Flags<[NoArgumentUnused, HelpHidden]>, diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 843d68c85bc59..0f1141ed8bcd9 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -7789,17 +7789,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, options::OPT_mno_amdgpu_ieee); } - // For all the host OpenMP offloading compile jobs we need to pass the targets - // information using -fopenmp-targets= option. - if (JA.isHostOffloading(Action::OFK_OpenMP)) { -SmallString<128> Targets("-fopenmp-targets="); - -SmallVector Triples; -auto TCRange = C.getOffloadToolChains(); -std::transform(TCRange.first, TCRange.second, std::back_inserter(Triples), - [](auto TC) { return TC.second->getTripleString(); }); -CmdArgs.push_back(Args.MakeArgString(Targets + llvm::join(Triples, ","))); - } + addOpenMPHostOffloadingArgs(C, JA, Args, CmdArgs); bool VirtualFunctionElimination = Args.hasFlag(options::OPT_fvirtual_function_elimination, diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 3d0714286139d..6e9744607d9eb 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1244,6 +1244,25 @@ bool tools::addOpenMPRuntime(const Compilation &C, ArgStringList &CmdArgs, return true; } +void tools::addOpenMPHostOffloadingArgs(const Compilation &C, +const JobAction &JA, +const llvm::opt::ArgList &Args, +llvm::opt::ArgStringList &CmdArgs) { + if (!JA.isHostOffloading(Action::OFK_OpenMP)) +return; + + // For all the host OpenMP offloading compile jobs we need to pass the targets + // information using -fopenmp-targets= option. + constexpr llvm::StringLiteral Targets("-fopenmp-targets="); + + SmallVector Triples; + auto TCRange = C.getOffloadToolChains(); + std::transform(TCRange.first, TCRange.second, std::back_inserter(Triples), + [](auto TC) { return TC.second->getTripleString(); }); + CmdArgs.push_back( + Args.MakeArgString(Twine(Targets) + llvm::join(Triples, ","))); +} + /// Add Fortran runtime libs void tools::addFortranRuntimeLibs(const ToolChain &TC, const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { diff --git a/clang/lib/Driver/ToolChains/CommonArgs.h b/clang/lib/Driver/ToolChains/CommonArgs.h index 0d3b4d6b783ba..0c97398dfcfa3 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.h +++ b/clang/lib/Driver/ToolChains/CommonArgs.h @@ -119,6 +119,11 @@ bool addOpenMPRuntime(const Compilation &C, llvm::opt::ArgStringList &CmdArgs, bool ForceStaticHostRuntime = false, bool IsOffloadingHost = false, bool GompNeedsRT = false); +/// Adds offloading options for OpenMP host compilation to
[clang] [flang] [Flang][Driver] Introduce -fopenmp-targets offloading option (PR #100152)
https://github.com/skatrak updated https://github.com/llvm/llvm-project/pull/100152 >From e59c38db58ba5ca2eef66d3b3477d5ad81043228 Mon Sep 17 00:00:00 2001 From: Sergio Afonso Date: Tue, 23 Jul 2024 16:19:55 +0100 Subject: [PATCH 1/2] [Flang][Driver] Introduce -fopenmp-targets offloading option This patch modifies the flang driver to introduce the `-fopenmp-targets` option to the frontend compiler invocations corresponding to the OpenMP host device on offloading-enabled compilations. This option holds the list of offloading triples associated to the compilation and is used by clang to determine whether offloading calls should be generated for the host. --- clang/include/clang/Driver/Options.td | 2 +- clang/lib/Driver/ToolChains/Clang.cpp | 12 +--- clang/lib/Driver/ToolChains/CommonArgs.cpp | 19 +++ clang/lib/Driver/ToolChains/CommonArgs.h | 5 + clang/lib/Driver/ToolChains/Flang.cpp | 2 ++ flang/test/Driver/omp-driver-offload.f90 | 18 ++ 6 files changed, 46 insertions(+), 12 deletions(-) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index c8c56dbb51b28..59c6bb70d75a2 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3532,7 +3532,7 @@ def fopenmp_use_tls : Flag<["-"], "fopenmp-use-tls">, Group, def fnoopenmp_use_tls : Flag<["-"], "fnoopenmp-use-tls">, Group, Flags<[NoArgumentUnused, HelpHidden]>, Visibility<[ClangOption, CC1Option]>; def fopenmp_targets_EQ : CommaJoined<["-"], "fopenmp-targets=">, - Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option, FlangOption]>, + Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>, HelpText<"Specify comma-separated list of triples OpenMP offloading targets to be supported">; def fopenmp_relocatable_target : Flag<["-"], "fopenmp-relocatable-target">, Group, Flags<[NoArgumentUnused, HelpHidden]>, diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 843d68c85bc59..0f1141ed8bcd9 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -7789,17 +7789,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, options::OPT_mno_amdgpu_ieee); } - // For all the host OpenMP offloading compile jobs we need to pass the targets - // information using -fopenmp-targets= option. - if (JA.isHostOffloading(Action::OFK_OpenMP)) { -SmallString<128> Targets("-fopenmp-targets="); - -SmallVector Triples; -auto TCRange = C.getOffloadToolChains(); -std::transform(TCRange.first, TCRange.second, std::back_inserter(Triples), - [](auto TC) { return TC.second->getTripleString(); }); -CmdArgs.push_back(Args.MakeArgString(Targets + llvm::join(Triples, ","))); - } + addOpenMPHostOffloadingArgs(C, JA, Args, CmdArgs); bool VirtualFunctionElimination = Args.hasFlag(options::OPT_fvirtual_function_elimination, diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 3d0714286139d..6e9744607d9eb 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1244,6 +1244,25 @@ bool tools::addOpenMPRuntime(const Compilation &C, ArgStringList &CmdArgs, return true; } +void tools::addOpenMPHostOffloadingArgs(const Compilation &C, +const JobAction &JA, +const llvm::opt::ArgList &Args, +llvm::opt::ArgStringList &CmdArgs) { + if (!JA.isHostOffloading(Action::OFK_OpenMP)) +return; + + // For all the host OpenMP offloading compile jobs we need to pass the targets + // information using -fopenmp-targets= option. + constexpr llvm::StringLiteral Targets("-fopenmp-targets="); + + SmallVector Triples; + auto TCRange = C.getOffloadToolChains(); + std::transform(TCRange.first, TCRange.second, std::back_inserter(Triples), + [](auto TC) { return TC.second->getTripleString(); }); + CmdArgs.push_back( + Args.MakeArgString(Twine(Targets) + llvm::join(Triples, ","))); +} + /// Add Fortran runtime libs void tools::addFortranRuntimeLibs(const ToolChain &TC, const ArgList &Args, llvm::opt::ArgStringList &CmdArgs) { diff --git a/clang/lib/Driver/ToolChains/CommonArgs.h b/clang/lib/Driver/ToolChains/CommonArgs.h index 0d3b4d6b783ba..0c97398dfcfa3 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.h +++ b/clang/lib/Driver/ToolChains/CommonArgs.h @@ -119,6 +119,11 @@ bool addOpenMPRuntime(const Compilation &C, llvm::opt::ArgStringList &CmdArgs, bool ForceStaticHostRuntime = false, bool IsOffloadingHost = false, bool GompNeedsRT = false); +/// Adds offloading options for OpenMP host compilatio
[clang] [flang] [Flang][Driver] Introduce -fopenmp-targets offloading option (PR #100152)
https://github.com/skatrak closed https://github.com/llvm/llvm-project/pull/100152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [mlir] [MLIR][OpenMP] Add omp.target_triples attribute to the OffloadModuleInterface (PR #100154)
https://github.com/skatrak edited https://github.com/llvm/llvm-project/pull/100154 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits