================
@@ -3173,19 +3173,14 @@ convertOmpThreadprivate(Operation &opInst, 
llvm::IRBuilderBase &builder,
   LLVM::GlobalOp global =
       addressOfOp.getGlobal(moduleTranslation.symbolTable());
   llvm::GlobalValue *globalValue = moduleTranslation.lookupGlobal(global);
-
-  if (!ompBuilder->Config.isTargetDevice()) {
-    llvm::Type *type = globalValue->getValueType();
-    llvm::TypeSize typeSize =
-        
builder.GetInsertBlock()->getModule()->getDataLayout().getTypeStoreSize(
-            type);
-    llvm::ConstantInt *size = builder.getInt64(typeSize.getFixedValue());
-    llvm::Value *callInst = ompBuilder->createCachedThreadPrivate(
-        ompLoc, globalValue, size, global.getSymName() + ".cache");
-    moduleTranslation.mapValue(opInst.getResult(0), callInst);
-  } else {
-    moduleTranslation.mapValue(opInst.getResult(0), globalValue);
----------------
skatrak wrote:

To the best of my knowledge, the reason a case was added for target device here 
initially was that `omp.threadprivate` operations in a function containing a 
target region were being translated, even though they weren't located inside of 
a target region. If we didn't map their result to anything in LLVM here, then 
its uses in e.g. map clauses would crash the compiler. It's close to the same 
reason why you implemented the "scope" translation of some OpenMP ops as well.

Since the previous PR in the stack would remove these host `omp.threadprivate` 
ops and replace them with placeholders, the consequence is that we don't have 
to do this anymore. If the resulting value originally was mapped into the 
target region then, indeed, it would come in as a kernel argument.

I'm not sure if this somehow breaks translation of `threadprivate` ops residing 
inside of target regions and device functions, though. Perhaps @agozillon can 
comment if this change might inadvertently break actual lowering of 
`threadprivate` for the device, as he originally added this logic here. Or 
maybe `threadprivate` didn't work before in the target device and still 
doesn't, which is also possible.

https://github.com/llvm/llvm-project/pull/137201
_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

Reply via email to