[PATCH] D134662: [OpenMPIRBuilder] Migrate emitOffloadingArraysArgument from clang

[Jan Sjödin via Phabricator via llvm-commits]

jsjodin marked 5 inline comments as done.
jsjodin added a comment.

Updated the code for everthing except for type creation, which will be fixed shortly.



================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1044
+    //    llvm::DenseMap<const ValueDecl *, Address> CaptureDeviceAddrMap;
+    llvm::DenseMap<const void *, Value*> CaptureDeviceAddrMap; // FIXME
+
----------------
jdoerfert wrote:
> Seems unused. If needed in can live in Clang.
Yes, this is dead code I forgot to remove.


================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1053-1059
+      RTArgs.BasePointersArray = nullptr;
+      RTArgs.PointersArray = nullptr;
+      RTArgs.SizesArray = nullptr;
+      RTArgs.MapTypesArray = nullptr;
+      RTArgs.MapTypesArrayEnd = nullptr;
+      RTArgs.MapNamesArray = nullptr;
+      RTArgs.MappersArray = nullptr;
----------------
jdoerfert wrote:
> RTArgs = TargetDataRTArgs()?
Yes, much better.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:3997-4001
+  auto Int8Ty = Type::getInt8Ty(M.getContext());
+  auto VoidPtrTy =  Int8Ty->getPointerTo(0);
+  auto VoidPtrPtrTy = VoidPtrTy->getPointerTo(0);
+  auto Int64Ty = Type::getInt64Ty(M.getContext());
+  auto Int64PtrTy = Int64Ty->getPointerTo();
----------------
jdoerfert wrote:
> These types should exist but I guess this is OK.
It can be compressed by using PointerType *VoidPtrType = Type::getInt8PtrTy(C); I will also fix this, but in the next patch update.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4002
+  auto Int64PtrTy = Int64Ty->getPointerTo();
+  if (Info.NumberOfPtrs) {
+    RTArgs.BasePointersArray = Builder.CreateConstInBoundsGEP2_32(
----------------
jdoerfert wrote:
> Flip the cases, simple first, then exit.
Did an early return.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4026
+    // requested.
+    if (EmitDebug)
+      RTArgs.MapNamesArray = ConstantPointerNull::get(VoidPtrPtrTy);
----------------
jdoerfert wrote:
> !EmitDebug
Test and call site were both swapped, fixed both.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134662/new/

https://reviews.llvm.org/D134662