[Mlir-commits] [llvm] [mlir] [OpenMP][MLIR] Fix threadprivate lowering when compiling for target when target operations are in use (PR #119310)

[Sergio Afonso]

================
@@ -2537,24 +2537,34 @@ convertOmpThreadprivate(Operation &opInst, llvm::IRBuilderBase &builder,
 
   Value symAddr = threadprivateOp.getSymAddr();
   auto *symOp = symAddr.getDefiningOp();
+
+  if (auto asCast = dyn_cast<LLVM::AddrSpaceCastOp>(symOp))
+    symOp = asCast.getOperand().getDefiningOp();
+
   if (!isa<LLVM::AddressOfOp>(symOp))
     return opInst.emitError("Addressing symbol not found");
   LLVM::AddressOfOp addressOfOp = dyn_cast<LLVM::AddressOfOp>(symOp);
 
   LLVM::GlobalOp global =
       addressOfOp.getGlobal(moduleTranslation.symbolTable());
   llvm::GlobalValue *globalValue = moduleTranslation.lookupGlobal(global);
-  llvm::Type *type = globalValue->getValueType();
-  llvm::TypeSize typeSize =
-      builder.GetInsertBlock()->getModule()->getDataLayout().getTypeStoreSize(
-          type);
-  llvm::ConstantInt *size = builder.getInt64(typeSize.getFixedValue());
-  llvm::StringRef suffix = llvm::StringRef(".cache", 6);
-  std::string cacheName = (Twine(global.getSymName()).concat(suffix)).str();
-  llvm::Value *callInst =
-      moduleTranslation.getOpenMPBuilder()->createCachedThreadPrivate(
-          ompLoc, globalValue, size, cacheName);
-  moduleTranslation.mapValue(opInst.getResult(0), callInst);
+
+  if (!moduleTranslation.getOpenMPBuilder()->Config.isTargetDevice()) {
+    llvm::Type *type = globalValue->getValueType();
+    llvm::TypeSize typeSize =
+        builder.GetInsertBlock()->getModule()->getDataLayout().getTypeStoreSize(
+            type);
+    llvm::ConstantInt *size = builder.getInt64(typeSize.getFixedValue());
+    llvm::StringRef suffix = llvm::StringRef(".cache", 6);
+    std::string cacheName = (Twine(global.getSymName()).concat(suffix)).str();
+    llvm::Value *callInst =
+        moduleTranslation.getOpenMPBuilder()->createCachedThreadPrivate(
+            ompLoc, globalValue, size, cacheName);
----------------
skatrak wrote:

Nit: Isn't this refactoring possible? I know this patch isn't changing this, but the implementation looks unnecessarily convoluted.
```suggestion
    llvm::Value *callInst =
        ompBuilder->createCachedThreadPrivate(
            ompLoc, globalValue, size, global.getSymName() + ".cache");
```

https://github.com/llvm/llvm-project/pull/119310