[llvm] [flang] [clang] [NFC][AMDGPU] Move address space enum to LLVM directory (PR #73944)

2023-12-05 Thread Sergio Afonso via cfe-commits

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)

2023-11-17 Thread Sergio Afonso via cfe-commits

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)

2023-11-17 Thread Sergio Afonso via cfe-commits

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)

2023-11-17 Thread Sergio Afonso via cfe-commits


@@ -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)

2023-11-17 Thread Sergio Afonso via cfe-commits


@@ -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)

2023-11-20 Thread Sergio Afonso via cfe-commits


@@ -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)

2023-11-20 Thread Sergio Afonso via cfe-commits

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)

2023-11-20 Thread Sergio Afonso via cfe-commits


@@ -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)

2023-11-20 Thread Sergio Afonso via cfe-commits

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)

2023-11-30 Thread Sergio Afonso via cfe-commits

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)

2023-11-30 Thread Sergio Afonso via cfe-commits

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)

2023-11-30 Thread Sergio Afonso via cfe-commits


@@ -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)

2023-11-30 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-03-04 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-02-05 Thread Sergio Afonso via cfe-commits

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)

2024-02-05 Thread Sergio Afonso via cfe-commits

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)

2024-02-05 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-02-05 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-02-05 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-02-06 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-02-06 Thread Sergio Afonso via cfe-commits

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)

2024-01-15 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-01-15 Thread Sergio Afonso via cfe-commits

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)

2024-01-15 Thread Sergio Afonso via cfe-commits

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)

2024-01-15 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-01-18 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-01-18 Thread Sergio Afonso via cfe-commits

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)

2024-07-03 Thread Sergio Afonso via cfe-commits

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)

2024-07-03 Thread Sergio Afonso via cfe-commits

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)

2024-07-03 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-07-03 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-06-13 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-06-04 Thread Sergio Afonso via cfe-commits

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)

2024-06-05 Thread Sergio Afonso via cfe-commits

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)

2024-06-05 Thread Sergio Afonso via cfe-commits

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)

2024-06-05 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-06-05 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-06-05 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-06-05 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-06-05 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-06-05 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-06-05 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-06-05 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-06-05 Thread Sergio Afonso via cfe-commits

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)

2024-06-05 Thread Sergio Afonso via cfe-commits

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)

2024-06-05 Thread Sergio Afonso via cfe-commits

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)

2024-06-26 Thread Sergio Afonso via cfe-commits

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)

2024-06-26 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-06-26 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-06-26 Thread Sergio Afonso via cfe-commits

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)

2024-06-26 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-06-26 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-06-26 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-06-26 Thread Sergio Afonso via cfe-commits


@@ -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

2023-09-14 Thread Sergio Afonso via cfe-commits

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

2023-09-14 Thread Sergio Afonso via cfe-commits

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

2023-08-07 Thread Sergio Afonso via cfe-commits

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""

2023-08-10 Thread Sergio Afonso via cfe-commits

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

2023-03-24 Thread Sergio Afonso via cfe-commits

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

2023-04-12 Thread Sergio Afonso via cfe-commits

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)

2024-07-23 Thread Sergio Afonso via cfe-commits

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)

2024-07-23 Thread Sergio Afonso via cfe-commits

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)

2024-07-24 Thread Sergio Afonso via cfe-commits

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)

2024-07-24 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-07-24 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-07-24 Thread Sergio Afonso via cfe-commits

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)

2024-07-25 Thread Sergio Afonso via cfe-commits

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)

2024-07-25 Thread Sergio Afonso via cfe-commits

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)

2024-07-26 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-07-29 Thread Sergio Afonso via cfe-commits

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)

2024-07-30 Thread Sergio Afonso via cfe-commits

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)

2024-07-30 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-07-30 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-07-30 Thread Sergio Afonso via cfe-commits


@@ -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)

2024-07-31 Thread Sergio Afonso via cfe-commits

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)

2024-07-31 Thread Sergio Afonso via cfe-commits

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)

2024-08-01 Thread Sergio Afonso via cfe-commits

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)

2024-08-01 Thread Sergio Afonso via cfe-commits

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)

2024-08-01 Thread Sergio Afonso via cfe-commits

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)

2024-08-01 Thread Sergio Afonso via cfe-commits

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