[MLIR][OpenMP] Initial Lowering of Declare Target for Data

[agozillon]

Summary

This patch adds initial lowering for DeclareTargetAttr on
GlobalOp's utilising registerTargetGlobalVariable
and getAddrOfDeclareTargetVar from the
OMPIRBuilder.

DeclareTargetAttr encapsulates in MLIR a declare
target declaration applied to a piece of global data
or a function and indicates it should undergo
special handling.

The patch currently does not factor in filtering
based on device_type clauses (e.g. no emission of
globals for device if host specified), this will come in
a future iteration.

[agozillon]

This patch depends on https://reviews.llvm.org/D149162 and https://reviews.llvm.org/D148370 alongside https://reviews.llvm.org/D146063 which will undergo some further updates tomorrow to support this lowering patch (changes to the Attribute for retaining more clause information and increased PST -> MLIR lowering support).

[agozillon]

• The traditional (for me) missing end of file newline

[agozillon]

mlir/test/Target/LLVMIR/openmp-llvm.mlir

2500

There's a slightly more frontend (Flang) oriented test that I'll add to https://reviews.llvm.org/D146063 which tests device lowering via Fortran -> LLVM-IR checking.

I am not too sure it's possible to test the device lowering from here as it requires a pre-made .bc file that matches the test to be specified as an attribute which isn't ideal, but I am also not too sure how to incorporate a filepath that can change based on location of the test into MLIR either e.g. filepath="someusersfolderpath/test.bc"

[agozillon]

• [Flang] Fix functions after rebase
• Add load generation for device and ref swap for host, and add device test

[agozillon]

• Remove the rebase artifact and add a newline at end of test

[agozillon]

If someone has time to review this patch it would be much appreciated, it's at a stage where the current changes can likely go in, if everyone is happy with them of course and it passes review. However, if we wish to wait a little longer until more kernel launch code is in that's also perfectly fine, but I'd still love to get this to a stage where we're happy with the way it works and it's ready to go in when the rest lands!

The most recent update completes the generation of the IR that allows declare target link and target op's map to be used in conjunction to pass data to and from the device (for simple cases at least, int's and arrays of ints, possibly other basic types, but I have yet to test with allocatables or complex types). It requires some additional glue code for target op that can be added in a seperate patch after https://reviews.llvm.org/D151035 has landed (the glue code is just generic combined info data generation that will need to be done for all map clauses). The update adds generation of additional Loads for device arguments, further maptype modification for map arguments based on if it's a declare target map item and swapping the base pointer to the appropriate reference pointer when the mapped variable is a declare target link variable. This is in addition to the generation of ref pointers and relevant metadata that was done by the patch prior to the recent update.

The current test depends on: https://reviews.llvm.org/D153781 as it's in a test that currently makes use of split-input-file (although, this is for neatness, not necessity, it can likely be altered to not use it or be in a seperate file)

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1382

This currently isn't tested in patch (I've tested it downstream though of course), it alters the host mapping, it requires the dependent patch here: https://reviews.llvm.org/D151035 and then possibly another small additional patch to link kernel argument generation up in OpenMPToLLVMIRtranslation.cpp

However, this patch can likely land without the dependents, this little segment just needs to have an additional test added later, which can be added alongside some proper execution tests when we have more of the kernel launch and argument generation in upstream.

[agozillon]

I would appreciate very much if a reviewer could get a spare minute to give this patch an initial lookover (the buildbot break is related to https://reviews.llvm.org/D153781 and can be worked around by removing the split-input-file option from the mlir-translate tool), thank you very much for your time

[kiranchandramohan]

Thanks. I have started going through the patch. I have one question about a deletion, see inline.

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1776–1784

Not clear whether I understand this. Haven't you used the global that was generated by MLIR llvm dialect translation for registering and getting the address of? So why does this need a deletion here?

mlir/test/Target/LLVMIR/omptarget-declare-target-llvm.mlir

4 ↗

(On Diff #534581)

Is this an LLVM IR attribute or an MLIR attribute that you are talking about here?

[kiranchandramohan]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1725

Nit: expand the type here.

1732–1733

Nit: expand type here.

1764

Nit: Could you tag the parameters with the name if they are constants or nullptr?

1775

Nit: Could you tag the parameters with the name if they are constants or nullptr?

[agozillon]

In D149368#4477490, @kiranchandramohan wrote:

Thanks. I have started going through the patch. I have one question about a deletion, see inline.

Thank you very much Kiran. I'll get to fixing the Nits hopefully tomorrow or at the beginning of next week. I've done my best to reply to your questions so far!

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1776–1784

For device this is not required and is effectively just dead code when this case is reached as the "real" global will not be used, but has unfortunately already been generated when we get to this stage. We could just not delete it and let some later llvm optimisation pass clean it up, if we wish to be very safe, or add it to a list for removal at the end of the module lowering.

On device for cases such as declare target link(some_int), some_int is effectively just a global null pointer that the OpenMP runtime will "magically" read/write the data into as necessary and the device can access it through, this global being deleted is seperate from it (this is just my understanding, someone with more OpenMP knowledge can possibly elaborate further on the details or disprove my understanding). The call to getAddrOfDeclareTargetVar will generate something like the following in this case:

@_some_int_decl_tgt_ref_ptr = weak global ptr nullptr

And then the device kernel will use the pointer to access the data, we simply need the globals name, to keep it synchronized across device and host and so we know it refers to the same data!

mlir/test/Target/LLVMIR/omptarget-declare-target-llvm.mlir

4 ↗

(On Diff #534581)

In this case it's an MLIR attribute, the host-ir-file-path MLIR Attr applied to the module, it's required by the device pass and passed from the host pass after it's generated the .bc file for the host segment of the end binary. It's required to populate the OpenMPIRBuilder with the necessary information for it to generate the OpenMP metadata for kernels and declare target and perhaps some other things so that the runtime can appropriately manage data and kernels (at least from my understanding so far). It's primarily required because we must generate synchronized metadata across both the host and device pass, Clang does the same thing at the moment a side affect of multiple passes I believe as SYCL does something similar.

In theory you could have a seperate device version of this test, that will compile this host test from an mlir file to .bc and then use the generated .bc file in the test, but I don't think it'd work as we have to then generate the MLIR attribute that holds the file path on the fly, unless it's possible to always guarantee where the file will be placed (can't really predict a users directory structure for example, although, perhaps you can just specify it relative from the test file though) as then you can just hard-code the attribute with the fixed filepath.

[agozillon]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1776–1784

Slight amend: Some other information than the name is needed from the global as well, such as llvm type etc. to generate the ref ptr for device I believe, so it is required up until this point I think as we don't have the same access to this information as Clang. But perhaps it's entirely retrievable from the MLIR type and value.

[agozillon]

• Apply the nit comments from reviewer

[agozillon]

I tried to address your Nit's in the last commit!

[agozillon]

Rebase, move flag changing from PFT lowering, to MLIR -> LLVM-IR lowering, teach early outliner pass to handle declare target globals, add a new runtime test now that enough is in place for one

[agozillon]

A small ping for a review of this patch, breathing some new life into this as there's now enough upstream for the patch to fully function and have a runtime test. Some minor changes to the patch mentioned briefly in the previous message, so hopefully we can get this to a state we're happy with before the end of the month and the full swap to Github!

flang/lib/Optimizer/Transforms/OMPEarlyOutlining.cpp

100 ↗

(On Diff #556737)

These changes are a slightly regressed and simplified version of those in: https://reviews.llvm.org/D158735 depending on which lands first a rebase will be required, but it shouldn't be a problem.

I wish/wished to keep this patch self reliant and out of the more complex stack of patches as there's enough upstream to support this and it's a very different piece of functionality and landing all 5 in tandem could become rather complex if there's buildbot failures!

[agozillon]

A small ping on this patch for some attention please if possible, it'd be nice to push it closer to acceptance if at all feasible!

[kiranchandramohan]

@skatrak could you help review this?

[skatrak]

Thank you Andrew for this work! I've got a few comments, but most of them are just style-related or pointing out typos.

flang/lib/Optimizer/Transforms/OMPEarlyOutlining.cpp

32 ↗

(On Diff #556737)

Maybe rename this to isAddressOfGlobalDeclareTarget or similar, pass in the mlir::Value and check for the defining op inside this function. It should reduce some code duplication among the two calls to this function and also make its purpose clearer.

33 ↗

(On Diff #556737)

Nit: Perhaps for consistency I would swap out fir::AddrOfOp and fir::GlobalOp in the declared variable names here for auto, and replace auto for mlir::Value in the two new loops below over inputs and targetOp.getMapOperands(), so that you only use auto where the type is already spelled out in the initialization.

94 ↗

(On Diff #556737)

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1384

Nit: It might be more readable to structure it as:

if (refPtr)
  mapFlag |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PTR_AND_OBJ;
else if (isTargetParams)
  mapFlag |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TARGET_PARAM;

But feel free to ignore if you disagree.

1400

Nit: It should be fine to pass by value here.

1405

1406

The two calls to getDefiningOp I think can be simplified like this.

1409

This should be inside of an 'if' statement, since the returned value is accessed right after with no presence checks.

1419

I think this check is redundant. By comparing the result of declareTargetGlobal.getDeclareTargetCaptureClause() against some non-null values we're already making sure that the condition won't be satisfied if there is no omp.declare_target attribute.

1425

Can you refactor this suffix generation code into a function? I think it would help with readability here.

1629

I think llvm::ArrayRef<mlir::Value> is better suited here.

1632

1636

1638

1643

1653

According to the programmer's manual here, it is preferred to avoid hardcoding the "small" size of small vectors when used as output parameters.

1655

I think it's more readable to negate the condition rather than adding a continue statement here, but feel free to ignore this comment if you disagree.

1682

1683–1780

Is there any reason to make this function visible outside the translation unit?

1706

1710

This seems like it would crash if targetTripleAttr was not present or a StringAttr, since the output of dyn_cast_or_null is dereferenced without checking first.

1714

Can you provide a default to fail gracefully if loc turns out to be null? Or throw out an error earlier when it's initialized, if that's not possible.

1720

Please add comments with the parameter names for the ones set to false or nullptr here and in the call below to getAddrOfDeclareTargetVar().

[agozillon]

• Update based on reviewer comments and rebasing

[agozillon]

Made an attempt at addressing all review points made by @skatrak in the prior patch as well as rebase on most recent upstream changes. Let me know what you think when you have a spare moment @skatrak! Thank you very much for your review!

flang/lib/Optimizer/Transforms/OMPEarlyOutlining.cpp

33 ↗

(On Diff #556737)

rebased on the recent map changes I upstreamed, so the changeset here isn't required anymore! but I've taken into account these review points and done my best to apply them none the less, as they're good suggestions for tidying it up a bit more.

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1683–1780

minor rebase issue on my end from when you added the function upstream before me I think! no reason for it not to be static thank you for the catch.

1714

The call to getTargetEntryUniqueInfo that uses it emits an error when it can't generate a unique id from the file data given, so I've attempted to emit some empty information in this case so that it fails gracefully (and silently) here and the error from the OMPIRBuilder function is emitted.

[agozillon]

• Update test to use only explicit variables (implicits don't work quite yet)

[agozillon]

openmp/libomptarget/test/offloading/fortran/declare-target-array-in-target-region.f90

19 ↗

(On Diff #557087)

I've changed this test to only use explicitly mapped variables, as implicitly mapped variables don't work quite yet, a little awkward looking but it works similarly to the previous iteration. I believe check-openmp/runtimes may have been picking up my flang build with downstream changes previously and giving a little bit of a false pass for the previous iteration, after rebasing it's picked up the right one though.

The implicit map handling should come soon, how we handle it depends on how the IsolatedFromAbove changes that @TIFitis is working on goes, as I think his current work will supersede the small pass I worked on for it and retire the EarlyOutlinigPass.

[skatrak]

Thanks Andrew for the changes. Just a couple of nits, but overall LGTM.

flang/lib/Optimizer/Transforms/OMPEarlyOutlining.cpp

44 ↗

(On Diff #557087)

Just like in the other spot I suggested the same change, this line and the next can be simplified like this.

180 ↗

(On Diff #557087)

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1380

If we can assert at this point that mapTypes can only contain IntegerAttr at this point, it's better to do mapTypes[index].cast<IntegerAttr>. Otherwise, we should check the result of the dyn_cast before calling getUInt on it to avoid a crash.

1420

I think we can remove const here: https://mlir.llvm.org/docs/Rationale/UsageOfConst/

1728

Seems like clang-format missed breaking this line

[agozillon]

• Fix reviewer comments

[agozillon]

I think I've covered all the review points you made in the last update, please let me know if I'm missing anything though!

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1380

It can only contain an IntegerAttr, a UInt to be precise in this case, otherwise there'd be complaints somewhere else in the compiler about mismatched types! So in this case I've swapped it to a cast.

[skatrak]

Thank you, LGTM!

[agozillon]

Thank you for the review and help @skatrak!

@kiranchandramohan would you like some time to review and approve the change set?

[kiranchandramohan]

In D149368#4648820, @agozillon wrote:

Thank you for the review and help @skatrak!

@kiranchandramohan would you like some time to review and approve the change set?

@skatrak has given a great review. Thanks @skatrak.

Please go ahead.

openmp/libomptarget/test/offloading/fortran/declare-target-array-in-target-region.f90

1 ↗

(On Diff #556737)

Nice test.

[agozillon]

@kiranchandramohan thank you! I'll land the patch in the next couple of hours then provided no one has any further feedback or input during that time period.

openmp/libomptarget/test/offloading/fortran/declare-target-array-in-target-region.f90

1 ↗

(On Diff #556737)

Thank you, it's a team effort, and it will hopefully be less convoluted to make tests in the near future after Akash's work.

Fortran has a lot of hidden generation of variables/constants that are generated outside of the target region but used inside of it, at least for the moment, that creates some hidden implicit map dependencies unfortunately e.g. a do loop used inside of a target region will create some implicit dependencies on the index variable, and loop constants defining its range (the former you can explicitly map, the latter not so much, although perhaps there's a Fortran syntax for a do loop that can allow for it that I don't know!). It'd be reasonably easy to teach the lowering to handle it better, but it'd be an interim work around, so I think it's better to wait for @TIFitis IsolatedFromAbove work so it can be handled correctly and concisely.

[kda]

Looks like this breaking ASAN related buildbots:
https://lab.llvm.org/buildbot/#/builders/5/builds/36821
https://lab.llvm.org/buildbot/#/builders/168/builds/15784

[agozillon]

In D149368#4649127, @kda wrote:

Looks like this breaking ASAN related buildbots:
https://lab.llvm.org/buildbot/#/builders/5/builds/36821
https://lab.llvm.org/buildbot/#/builders/168/builds/15784

Thank you, I'm looking into it. Any insights into it are appreciated :-)

[vzakhari]

flang/lib/Optimizer/Transforms/OMPEarlyOutlining.cpp

137 ↗

(On Diff #557133)

FWIW, doesn't this invalidate the underlying iterator?

[agozillon]

flang/lib/Optimizer/Transforms/OMPEarlyOutlining.cpp

137 ↗

(On Diff #557133)

it might actually, thank you for the catch.

[agozillon]

found the issue now working on a fix for it.

[agozillon]

should be fixed with this commit: https://github.com/llvm/llvm-project/commit/171d8c40288a995f7c34b0479f31241132af67a5 which tries to address @vzakhari's comment and the memory leak I inadvertently added (my apologies, and sorry for the annoying unrelated to your commits buildbot messages for the past few hours)

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

 
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/TypeSwitch.h"
 #include "llvm/Frontend/OpenMP/OMPConstants.h"
 #include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/TargetParser/Triple.h"
+
+#include <utility>
 
 using namespace mlir;
 
 namespace {
 static llvm::omp::ScheduleKind
 convertToScheduleKind(std::optional<omp::ClauseScheduleKind> schedKind) {
   if (!schedKind.has_value())
     return llvm::omp::OMP_SCHEDULE_Default;
static LogicalResult processMapOperand(
   llvm::ArrayType *arrI8PtrTy = llvm::ArrayType::get(i8PtrTy, numMapOperands);
   llvm::IntegerType *i64Ty = builder.getInt64Ty();
   llvm::ArrayType *arrI64Ty = llvm::ArrayType::get(i64Ty, numMapOperands);
 
   unsigned index = 0;
   for (const auto &mapOp : mapOperands) {
     const auto &mapTypeOp = mapTypes[index];
 
     llvm::Value *mapOpValue = moduleTranslation.lookupValue(mapOp);
     llvm::Value *mapOpPtrBase;
     llvm::Value *mapOpPtr;
     llvm::Value *mapOpSize;
 
     if (mapOp.getType().isa<LLVM::LLVMPointerType>()) {
       mapOpPtrBase = mapOpValue;
       mapOpPtr = mapOpValue;
       mapOpSize = ompBuilder->getSizeInBytes(mapOpValue);
     } else {
       return failure();
     }
 
     // Store base pointer extracted from operand into the i-th position of
     // argBase.
     llvm::Value *ptrBaseGEP = builder.CreateInBoundsGEP(
         arrI8PtrTy, mapperAllocas.ArgsBase,
         {builder.getInt32(0), builder.getInt32(index)});
     llvm::Value *ptrBaseCast = builder.CreateBitCast(
         ptrBaseGEP, mapOpPtrBase->getType()->getPointerTo());
     builder.CreateStore(mapOpPtrBase, ptrBaseCast);
 
     // Store pointer extracted from operand into the i-th position of args.
     llvm::Value *ptrGEP = builder.CreateInBoundsGEP(
         arrI8PtrTy, mapperAllocas.Args,
         {builder.getInt32(0), builder.getInt32(index)});
     llvm::Value *ptrCast =
         builder.CreateBitCast(ptrGEP, mapOpPtr->getType()->getPointerTo());
     builder.CreateStore(mapOpPtr, ptrCast);
 
     // Store size extracted from operand into the i-th position of argSizes.
     llvm::Value *sizeGEP = builder.CreateInBoundsGEP(
         arrI64Ty, mapperAllocas.ArgSizes,
         {builder.getInt32(0), builder.getInt32(index)});
     builder.CreateStore(mapOpSize, sizeGEP);
 
     mapTypeFlags.push_back(mapTypeOp.dyn_cast<mlir::IntegerAttr>().getInt());
     llvm::Constant *mapName =
         mlir::LLVM::createMappingInformation(mapOp.getLoc(), *ompBuilder);
     mapNames.push_back(mapName);
     ++index;
   }
 
   return success();
 }
 
 static LogicalResult
 convertOmpTargetData(Operation *op, llvm::IRBuilderBase &builder,
                      LLVM::ModuleTranslation &moduleTranslation) {
   unsigned numMapOperands;
   llvm::Value *ifCond = nullptr;
   int64_t deviceID = llvm::omp::OMP_DEVICEID_UNDEF;
   SmallVector<Value> mapOperands;
   ArrayAttr mapTypes;
if (targetOp.getNowait()) {
     opInst.emitError("Nowait clause not yet supported");
     return false;
   }
 
   return true;
 }
 
 static LogicalResult
 convertOmpTarget(Operation &opInst, llvm::IRBuilderBase &builder,
                  LLVM::ModuleTranslation &moduleTranslation) {
 
   if (!targetOpSupported(opInst))
     return failure();
 
   bool isDevice = false;
   if (auto offloadMod = dyn_cast<mlir::omp::OffloadModuleInterface>(
           opInst.getParentOfType<mlir::ModuleOp>().getOperation())) {
     isDevice = offloadMod.getIsDevice();
   }
 
   if (isDevice) // TODO: Implement device codegen.
     return success();
 
   auto targetOp = cast<omp::TargetOp>(opInst);
   auto &targetRegion = targetOp.getRegion();
 
   llvm::SetVector<Value> operandSet;
   getUsedValuesDefinedAbove(targetRegion, operandSet);
 
   // Collect the input arguments.
   llvm::SmallVector<llvm::Value *> inputs;
   for (Value operand : operandSet)
     inputs.push_back(moduleTranslation.lookupValue(operand));
 
   LogicalResult bodyGenStatus = success();
 
   using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
   auto bodyCB = [&](InsertPointTy allocaIP,
                     InsertPointTy codeGenIP) -> InsertPointTy {
     builder.restoreIP(codeGenIP);
     llvm::BasicBlock *exitBlock = convertOmpOpRegions(
         targetRegion, "omp.target", builder, moduleTranslation, bodyGenStatus);
     builder.SetInsertPoint(exitBlock);
convertOmpTarget(Operation &opInst, llvm::IRBuilderBase &builder,
   int32_t defaultValTeams = -1;
   int32_t defaultValThreads = -1;
 
   builder.restoreIP(moduleTranslation.getOpenMPBuilder()->createTarget(
       ompLoc, builder.saveIP(), entryInfo, defaultValTeams, defaultValThreads,
       inputs, bodyCB));
 
   return bodyGenStatus;
 }
 
+LogicalResult
+convertDeclareTargetAttr(Operation *op, mlir::omp::DeclareTargetAttr attribute,
+                         LLVM::ModuleTranslation &moduleTranslation) {
+  // do a return for functions at the moment, may need specialised lowering
+  // later to optimise but for the moment they execute on device.
+  if (LLVM::LLVMFuncOp gOp = dyn_cast<LLVM::LLVMFuncOp>(op))
+    return success();
+
+  auto convertToDeviceClauseKind = [](mlir::omp::DeclareTargetAttr attr) {
+    switch (attr.getDeviceType().getValue()) {
+    case mlir::omp::DeclareTargetDeviceType::host:
+      return llvm::OffloadEntriesInfoManager::OMPTargetDeviceClauseHost;
+      break;
+    case mlir::omp::DeclareTargetDeviceType::nohost:
+      return llvm::OffloadEntriesInfoManager::OMPTargetDeviceClauseNoHost;
+      break;
+    case mlir::omp::DeclareTargetDeviceType::any:
+      return llvm::OffloadEntriesInfoManager::OMPTargetDeviceClauseAny;
+      break;
+    default:
+      return llvm::OffloadEntriesInfoManager::OMPTargetDeviceClauseNone;
+      break;
+    }
+  };
+
+  auto convertToCaptureClauseKind = [](mlir::omp::DeclareTargetAttr attr) {
+    switch (attr.getCaptureClause().getValue()) {
+    case mlir::omp::DeclareTargetCaptureClause::to:
+      return llvm::OffloadEntriesInfoManager::OMPTargetGlobalVarEntryTo;
+      break;
+    case mlir::omp::DeclareTargetCaptureClause::link:
+      return llvm::OffloadEntriesInfoManager::OMPTargetGlobalVarEntryLink;
+      break;
+    default:
+      return llvm::OffloadEntriesInfoManager::OMPTargetGlobalVarEntryNone;
+      break;
+    }
+  };
+
+  if (LLVM::GlobalOp gOp = dyn_cast<LLVM::GlobalOp>(op)) {
+    llvm::Module *llvmModule = moduleTranslation.getLLVMModule();
+    if (auto *gVal = llvmModule->getNamedValue(gOp.getSymName())) {
+      llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
+      bool isDeclaration = gOp.isDeclaration();
+      bool isExternallyVisible =
+          gVal->getVisibility() !=
+          llvm::GlobalValue::VisibilityTypes::HiddenVisibility;
+      auto loc = op->getLoc()->findInstanceOf<FileLineColLoc>();
+      llvm::StringRef filename = loc.getFilename().getValue();
+      uint64_t line = loc.getLine();
+      llvm::StringRef mangledName = gOp.getSymName();
+      auto captureClause = convertToCaptureClauseKind(attribute);
+      auto deviceClause = convertToDeviceClauseKind(attribute);
+      // unused for MLIR at the moment, required in Clang for book
+      // keeping
+      std::vector<llvm::GlobalVariable *> generatedRefs;
+
+      std::vector<llvm::Triple> targetTriple;
+      auto targetTripleAttr =
+          op->getParentOfType<mlir::ModuleOp>().getOperation()->getAttr(
+              LLVM::LLVMDialect::getTargetTripleAttrName());
+      if (targetTripleAttr)
+        targetTriple.emplace_back(
+            targetTripleAttr.dyn_cast_or_null<mlir::StringAttr>().data());
+
+      ompBuilder->registerTargetGlobalVariable(
+          captureClause, deviceClause, isDeclaration, isExternallyVisible,
+          ompBuilder->getTargetEntryUniqueInfo(filename, line), mangledName,
+          llvmModule, generatedRefs, false, targetTriple, nullptr, nullptr,
+          gVal->getType(), gVal);
+
+      if (ompBuilder->Config.isEmbedded() &&
+          (attribute.getCaptureClause().getValue() !=
+               mlir::omp::DeclareTargetCaptureClause::to ||
+           ompBuilder->Config.hasRequiresUnifiedSharedMemory())) {
+        ompBuilder->getAddrOfDeclareTargetVar(
+            captureClause, deviceClause, isDeclaration, isExternallyVisible,
+            ompBuilder->getTargetEntryUniqueInfo(filename, line), mangledName,
+            llvmModule, generatedRefs, false, targetTriple, gVal->getType(),
+            nullptr, nullptr);
+        // A global has already been generated by this stage, unlike Clang, so
+        // this needs to be specially removed here for device when we're
+        // anything but a To clause specified variable with no unified shared
+        // memory.
+        if (llvm::GlobalValue *llvmVal =
+                llvmModule->getNamedValue(mangledName)) {
+          llvmVal->removeFromParent();
+          llvmVal->dropAllReferences();
+        }
+      }
+    }
+  }
+
+  return success();
+}
+
 namespace {
 
 /// Implementation of the dialect interface that converts operations belonging
 /// to the OpenMP dialect to LLVM IR.
 class OpenMPDialectLLVMIRTranslationInterface
     : public LLVMTranslationDialectInterface {
 public:
   using LLVMTranslationDialectInterface::LLVMTranslationDialectInterface;
 
   /// Translates the given operation to LLVM IR using the provided IR builder
   /// and saving the state in `moduleTranslation`.
   LogicalResult
   convertOperation(Operation *op, llvm::IRBuilderBase &builder,

 LogicalResult OpenMPDialectLLVMIRTranslationInterface::amendOperation(
     Operation *op, NamedAttribute attribute,
     LLVM::ModuleTranslation &moduleTranslation) const {
 
   return llvm::TypeSwitch<Attribute, LogicalResult>(attribute.getValue())
       .Case([&](mlir::omp::FlagsAttr rtlAttr) {
         return convertFlagsAttr(op, rtlAttr, moduleTranslation);
       })
+      .Case([&](mlir::omp::DeclareTargetAttr dtAttr) {
+        return convertDeclareTargetAttr(op, dtAttr, moduleTranslation);
+      })
       .Default([&](Attribute attr) {
         // fall through for omp attributes that do not require lowering and/or
         // have no concrete definition and thus no type to define a case on
         return success();
       });
 
   return failure();
 }

mlir/test/Target/LLVMIR/openmp-llvm.mlir

 // -----
 
 // CHECK: @__omp_rtl_debug_kind = weak_odr hidden constant i32 0
 // CHECK: @__omp_rtl_assume_teams_oversubscription = weak_odr hidden constant i32 1
 // CHECK: @__omp_rtl_assume_threads_oversubscription = weak_odr hidden constant i32 0
 // CHECK: @__omp_rtl_assume_no_thread_state = weak_odr hidden constant i32 1
 // CHECK: @__omp_rtl_assume_no_nested_parallelism = weak_odr hidden constant i32 0
 module attributes {omp.flags = #omp.flags<assume_teams_oversubscription = true, assume_no_thread_state = true>} {}
+
+
+// -----
+
+// NOTE: This test unfortunately only checks the host side, as the device side is dependent 
+// on a *.bc file created by the host and appended as an attribute to the module.
+
+// CHECK-DAG: %struct.__tgt_offload_entry = type { ptr, ptr, i64, i32, i32 } 
+// CHECK-DAG: !omp_offload.info = !{!{{.*}}}
+module attributes {llvm.target_triple = "x86_64-unknown-linux-gnu", omp.is_device = #omp.isdevice<is_device = false>} {
+
+  // CHECK-DAG: @_QMtest_0Earray_1d = global [3 x i32] [i32 1, i32 2, i32 3]
+  // CHECK-DAG: @_QMtest_0Earray_1d_decl_tgt_ref_ptr = weak global ptr @_QMtest_0Earray_1d 
+  // CHECK-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [36 x i8] c"_QMtest_0Earray_1d_decl_tgt_ref_ptr\00"
+  // CHECK-DAG: @.omp_offloading.entry._QMtest_0Earray_1d_decl_tgt_ref_ptr = weak constant %struct.__tgt_offload_entry { ptr @_QMtest_0Earray_1d_decl_tgt_ref_ptr, ptr @.omp_offloading.entry_name{{.*}}, i64 8, i32 1, i32 0 }, section "omp_offloading_entries", align 1
+  // CHECK-DAG: !{{.*}} = !{i32 {{.*}}, !"_QMtest_0Earray_1d_decl_tgt_ref_ptr", i32 {{.*}}, i32 {{.*}}}
+  llvm.mlir.global external @_QMtest_0Earray_1d(dense<[1, 2, 3]> : tensor<3xi32>) {addr_space = 0 : i32, omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (link)>} : !llvm.array<3 x i32>
+
+  // CHECK-DAG: @_QMtest_0Earray_2d = global [2 x [2 x i32]] {{.*}}
+  // CHECK-DAG: @_QMtest_0Earray_2d_decl_tgt_ref_ptr = weak global ptr @_QMtest_0Earray_2d 
+  // CHECK-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [36 x i8] c"_QMtest_0Earray_2d_decl_tgt_ref_ptr\00"
+  // CHECK-DAG: @.omp_offloading.entry._QMtest_0Earray_2d_decl_tgt_ref_ptr = weak constant %struct.__tgt_offload_entry { ptr @_QMtest_0Earray_2d_decl_tgt_ref_ptr, ptr @.omp_offloading.entry_name{{.*}}, i64 8, i32 1, i32 0 }, section "omp_offloading_entries", align 1
+  // CHECK-DAG: !{{.*}} = !{i32 {{.*}}, !"_QMtest_0Earray_2d_decl_tgt_ref_ptr", i32 {{.*}}, i32 {{.*}}}
+  llvm.mlir.global external @_QMtest_0Earray_2d() {addr_space = 0 : i32, omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (link)>} : !llvm.array<2 x array<2 x i32>> {
+    %0 = llvm.mlir.undef : !llvm.array<2 x array<2 x i32>>
+    %1 = llvm.mlir.constant(1 : i32) : i32
+    %2 = llvm.insertvalue %1, %0[0, 0] : !llvm.array<2 x array<2 x i32>> 
+    %3 = llvm.mlir.constant(2 : i32) : i32
+    %4 = llvm.insertvalue %3, %2[0, 1] : !llvm.array<2 x array<2 x i32>> 
+    %5 = llvm.mlir.constant(3 : i32) : i32
+    %6 = llvm.insertvalue %5, %4[1, 0] : !llvm.array<2 x array<2 x i32>> 
+    %7 = llvm.mlir.constant(4 : i32) : i32
+    %8 = llvm.insertvalue %7, %6[1, 1] : !llvm.array<2 x array<2 x i32>> 
+    %9 = llvm.mlir.constant(2 : index) : i64
+    %10 = llvm.mlir.constant(2 : index) : i64
+    llvm.return %8 : !llvm.array<2 x array<2 x i32>>
+  }
+
+  // CHECK-DAG:  @_QMtest_0Edata_extended_link_1 = global float 2.000000e+00
+  // CHECK-DAG:  @_QMtest_0Edata_extended_link_1_decl_tgt_ref_ptr = weak global ptr @_QMtest_0Edata_extended_link_1
+  // CHECK-DAG:  @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [48 x i8] c"_QMtest_0Edata_extended_link_1_decl_tgt_ref_ptr\00"
+  // CHECK-DAG:  @.omp_offloading.entry._QMtest_0Edata_extended_link_1_decl_tgt_ref_ptr = weak constant %struct.__tgt_offload_entry { ptr @_QMtest_0Edata_extended_link_1_decl_tgt_ref_ptr, ptr @.omp_offloading.entry_name{{.*}}, i64 8, i32 1, i32 0 }, section "omp_offloading_entries", align 1
+  // CHECK-DAG:  !{{.*}} = !{i32 {{.*}}, !"_QMtest_0Edata_extended_link_1_decl_tgt_ref_ptr", i32 {{.*}}, i32 {{.*}}}
+  llvm.mlir.global external @_QMtest_0Edata_extended_link_1() {addr_space = 0 : i32, omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (link)>} : f32 {
+    %0 = llvm.mlir.constant(2.000000e+00 : f32) : f32
+    llvm.return %0 : f32
+  }
+
+  // CHECK-DAG:  @_QMtest_0Edata_extended_link_2 = global float 3.000000e+00
+  // CHECK-DAG:  @_QMtest_0Edata_extended_link_2_decl_tgt_ref_ptr = weak global ptr @_QMtest_0Edata_extended_link_2
+  // CHECK-DAG:  @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [48 x i8] c"_QMtest_0Edata_extended_link_2_decl_tgt_ref_ptr\00"
+  // CHECK-DAG:  @.omp_offloading.entry._QMtest_0Edata_extended_link_2_decl_tgt_ref_ptr = weak constant %struct.__tgt_offload_entry { ptr @_QMtest_0Edata_extended_link_2_decl_tgt_ref_ptr, ptr @.omp_offloading.entry_name{{.*}}, i64 8, i32 1, i32 0 }, section "omp_offloading_entries", align 1
+  // CHECK-DAG:  !{{.*}} = !{i32 {{.*}}, !"_QMtest_0Edata_extended_link_2_decl_tgt_ref_ptr", i32 {{.*}}, i32 {{.*}}}
+  llvm.mlir.global external @_QMtest_0Edata_extended_link_2() {addr_space = 0 : i32, omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (link)>} : f32 {
+    %0 = llvm.mlir.constant(3.000000e+00 : f32) : f32
+    llvm.return %0 : f32
+  }
+
+  // CHECK-DAG: @_QMtest_0Edata_extended_to_1 = global float 2.000000e+00
+  // CHECK-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [29 x i8] c"_QMtest_0Edata_extended_to_1\00"
+  // CHECK-DAG: @.omp_offloading.entry._QMtest_0Edata_extended_to_1 = weak constant %struct.__tgt_offload_entry { ptr @_QMtest_0Edata_extended_to_1, ptr @.omp_offloading.entry_name{{.*}}, i64 4, i32 0, i32 0 }, section "omp_offloading_entries", align 1
+  // CHECK-DAG: !{{.*}} = !{i32 {{.*}}, !"_QMtest_0Edata_extended_to_1", i32 {{.*}}, i32 {{.*}}}
+  llvm.mlir.global external @_QMtest_0Edata_extended_to_1() {addr_space = 0 : i32, omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (to)>} : f32 {
+    %0 = llvm.mlir.constant(2.000000e+00 : f32) : f32
+    llvm.return %0 : f32
+  }
+
+  // CHECK-DAG: @_QMtest_0Edata_extended_to_2 = global float 3.000000e+00
+  // CHECK-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [29 x i8] c"_QMtest_0Edata_extended_to_2\00"
+  // CHECK-DAG: @.omp_offloading.entry._QMtest_0Edata_extended_to_2 = weak constant %struct.__tgt_offload_entry { ptr @_QMtest_0Edata_extended_to_2, ptr @.omp_offloading.entry_name{{.*}}, i64 4, i32 0, i32 0 }, section "omp_offloading_entries", align 1
+  // CHECK-DAG: !{{.*}} = !{i32 {{.*}}, !"_QMtest_0Edata_extended_to_2", i32 {{.*}}, i32 {{.*}}}
+  llvm.mlir.global external @_QMtest_0Edata_extended_to_2() {addr_space = 0 : i32, omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (to)>} : f32 {
+    %0 = llvm.mlir.constant(3.000000e+00 : f32) : f32
+    llvm.return %0 : f32
+  }
+
+  // CHECK-DAG: @_QMtest_0Edata_int = global i32 1
+  // CHECK-DAG: @_QMtest_0Edata_int_decl_tgt_ref_ptr = weak global ptr @_QMtest_0Edata_int 
+  // CHECK-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [36 x i8] c"_QMtest_0Edata_int_decl_tgt_ref_ptr\00"
+  // CHECK-DAG: @.omp_offloading.entry._QMtest_0Edata_int_decl_tgt_ref_ptr = weak constant %struct.__tgt_offload_entry { ptr @_QMtest_0Edata_int_decl_tgt_ref_ptr, ptr @.omp_offloading.entry_name{{.*}}, i64 8, i32 1, i32 0 }, section "omp_offloading_entries", align 1
+  // CHECK-DAG: !{{.*}} = !{i32 {{.*}}, !"_QMtest_0Edata_int_decl_tgt_ref_ptr", i32 {{.*}}, i32 {{.*}}}
+  llvm.mlir.global external @_QMtest_0Edata_int() {addr_space = 0 : i32, omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (link)>} : i32 {
+    %0 = llvm.mlir.constant(10 : i32) : i32
+    llvm.return %0 : i32
+  }
+
+  // CHECK-DAG: @_QMtest_0Edata_int_clauseless = global i32 1
+  // CHECK-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [30 x i8] c"_QMtest_0Edata_int_clauseless\00"
+  // CHECK-DAG: @.omp_offloading.entry._QMtest_0Edata_int_clauseless = weak constant %struct.__tgt_offload_entry { ptr @_QMtest_0Edata_int_clauseless, ptr @.omp_offloading.entry_name{{.*}}, i64 4, i32 0, i32 0 }, section "omp_offloading_entries", align 1
+  // CHECK-DAG: !{{.*}} = !{i32 {{.*}}, !"_QMtest_0Edata_int_clauseless", i32 {{.*}}, i32 {{.*}}}
+  llvm.mlir.global external @_QMtest_0Edata_int_clauseless() {addr_space = 0 : i32, omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (to)>} : i32 {
+    %0 = llvm.mlir.constant(1 : i32) : i32
+    llvm.return %0 : i32
+  }
+
+  // CHECK-DAG: @_QMtest_0Edata_int_to = global i32 5
+  // CHECK-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [22 x i8] c"_QMtest_0Edata_int_to\00"
+  // CHECK-DAG: @.omp_offloading.entry._QMtest_0Edata_int_to = weak constant %struct.__tgt_offload_entry { ptr @_QMtest_0Edata_int_to, ptr @.omp_offloading.entry_name{{.*}}, i64 4, i32 0, i32 0 }, section "omp_offloading_entries", align 1
+  // CHECK-DAG: !{{.*}} = !{i32 {{.*}}, !"_QMtest_0Edata_int_to", i32 {{.*}}, i32 {{.*}}}
+  llvm.mlir.global external @_QMtest_0Edata_int_to() {addr_space = 0 : i32, omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (to)>} : i32 {
+    %0 = llvm.mlir.constant(5 : i32) : i32
+    llvm.return %0 : i32
+  }
+
+  // CHECK-DAG: @_QMtest_0Ept1 = global { ptr, i64, i32, i8, i8, i8, i8 } { ptr null, i64 ptrtoint (ptr getelementptr (i32, ptr null, i32 1) to i64), i32 20180515, i8 0, i8 9, i8 1, i8 0 }
+  // CHECK-DAG: @_QMtest_0Ept1_decl_tgt_ref_ptr = weak global ptr @_QMtest_0Ept1
+  // CHECK-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [31 x i8] c"_QMtest_0Ept1_decl_tgt_ref_ptr\00"
+  // CHECK-DAG: @.omp_offloading.entry._QMtest_0Ept1_decl_tgt_ref_ptr = weak constant %struct.__tgt_offload_entry { ptr @_QMtest_0Ept1_decl_tgt_ref_ptr, ptr @.omp_offloading.entry_name{{.*}}, i64 8, i32 1, i32 0 }, section "omp_offloading_entries", align 1
+  // CHECK-DAG: !{{.*}} = !{i32 {{.*}}, !"_QMtest_0Ept1_decl_tgt_ref_ptr", i32 {{.*}}, i32 {{.*}}}
+  llvm.mlir.global external @_QMtest_0Ept1() {addr_space = 0 : i32, omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (link)>} : !llvm.struct<(ptr<i32>, i64, i32, i8, i8, i8, i8)> {
+    %0 = llvm.mlir.null : !llvm.ptr<i32>
+    %1 = llvm.mlir.constant(9 : i32) : i32
+    %2 = llvm.mlir.null : !llvm.ptr<i32>
+    %3 = llvm.getelementptr %2[1] : (!llvm.ptr<i32>) -> !llvm.ptr<i32>
+    %4 = llvm.ptrtoint %3 : !llvm.ptr<i32> to i64
+    %5 = llvm.mlir.undef : !llvm.struct<(ptr<i32>, i64, i32, i8, i8, i8, i8)>
+    %6 = llvm.insertvalue %4, %5[1] : !llvm.struct<(ptr<i32>, i64, i32, i8, i8, i8, i8)> 
+    %7 = llvm.mlir.constant(20180515 : i32) : i32
+    %8 = llvm.insertvalue %7, %6[2] : !llvm.struct<(ptr<i32>, i64, i32, i8, i8, i8, i8)> 
+    %9 = llvm.mlir.constant(0 : i32) : i32
+    %10 = llvm.trunc %9 : i32 to i8
+    %11 = llvm.insertvalue %10, %8[3] : !llvm.struct<(ptr<i32>, i64, i32, i8, i8, i8, i8)> 
+    %12 = llvm.trunc %1 : i32 to i8
+    %13 = llvm.insertvalue %12, %11[4] : !llvm.struct<(ptr<i32>, i64, i32, i8, i8, i8, i8)> 
+    %14 = llvm.mlir.constant(1 : i32) : i32
+    %15 = llvm.trunc %14 : i32 to i8
+    %16 = llvm.insertvalue %15, %13[5] : !llvm.struct<(ptr<i32>, i64, i32, i8, i8, i8, i8)> 
+    %17 = llvm.mlir.constant(0 : i32) : i32
+    %18 = llvm.trunc %17 : i32 to i8
+    %19 = llvm.insertvalue %18, %16[6] : !llvm.struct<(ptr<i32>, i64, i32, i8, i8, i8, i8)> 
+    %20 = llvm.bitcast %0 : !llvm.ptr<i32> to !llvm.ptr<i32>
+    %21 = llvm.insertvalue %20, %19[0] : !llvm.struct<(ptr<i32>, i64, i32, i8, i8, i8, i8)> 
+    llvm.return %21 : !llvm.struct<(ptr<i32>, i64, i32, i8, i8, i8, i8)>
+  }
+
+  // CHECK-DAG: @_QMtest_0Ept2_tar = global i32 5
+  // CHECK-DAG: @_QMtest_0Ept2_tar_decl_tgt_ref_ptr = weak global ptr @_QMtest_0Ept2_tar
+  // CHECK-DAG: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [35 x i8] c"_QMtest_0Ept2_tar_decl_tgt_ref_ptr\00"
+  // CHECK-DAG: @.omp_offloading.entry._QMtest_0Ept2_tar_decl_tgt_ref_ptr = weak constant %struct.__tgt_offload_entry { ptr @_QMtest_0Ept2_tar_decl_tgt_ref_ptr, ptr @.omp_offloading.entry_name{{.*}}, i64 8, i32 1, i32 0 }, section "omp_offloading_entries", align 1
+  // CHECK-DAG: !{{.*}} = !{i32 {{.*}}, !"_QMtest_0Ept2_tar_decl_tgt_ref_ptr", i32 {{.*}}, i32 {{.*}}}
+  llvm.mlir.global external @_QMtest_0Ept2_tar() {addr_space = 0 : i32, omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (link)>} : i32 {
+    %0 = llvm.mlir.constant(5 : i32) : i32
+    llvm.return %0 : i32
+  }
+}