[PATCH] D149872: [OpenMP][OMPIRBuilder] Migrate emitOffloadingArrays and EmitNonContiguousDescriptor from Clang

[Akash Banerjee via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3401a5f7584a: [OpenMP][OMPIRBuilder] Migrate 
emitOffloadingArrays and… (authored by TIFitis).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149872

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -4419,6 +4419,256 @@
 Builder.CreatePointerCast(Info.RTArgs.MappersArray, VoidPtrPtrTy);
 }
 
+void OpenMPIRBuilder::emitNonContiguousDescriptor(InsertPointTy AllocaIP,
+  InsertPointTy CodeGenIP,
+  MapInfosTy ,
+  TargetDataInfo ) {
+  MapInfosTy::StructNonContiguousInfo  =
+  CombinedInfo.NonContigInfo;
+
+  // Build an array of struct descriptor_dim and then assign it to
+  // offload_args.
+  //
+  // struct descriptor_dim {
+  //  uint64_t offset;
+  //  uint64_t count;
+  //  uint64_t stride
+  // };
+  Type *Int64Ty = Builder.getInt64Ty();
+  StructType *DimTy = StructType::create(
+  M.getContext(), ArrayRef({Int64Ty, Int64Ty, Int64Ty}),
+  "struct.descriptor_dim");
+
+  enum { OffsetFD = 0, CountFD, StrideFD };
+  // We need two index variable here since the size of "Dims" is the same as
+  // the size of Components, however, the size of offset, count, and stride is
+  // equal to the size of base declaration that is non-contiguous.
+  for (unsigned I = 0, L = 0, E = NonContigInfo.Dims.size(); I < E; ++I) {
+// Skip emitting ir if dimension size is 1 since it cannot be
+// non-contiguous.
+if (NonContigInfo.Dims[I] == 1)
+  continue;
+Builder.restoreIP(AllocaIP);
+ArrayType *ArrayTy = ArrayType::get(DimTy, NonContigInfo.Dims[I]);
+AllocaInst *DimsAddr =
+Builder.CreateAlloca(ArrayTy, /* ArraySize = */ nullptr, "dims");
+Builder.restoreIP(CodeGenIP);
+for (unsigned II = 0, EE = NonContigInfo.Dims[I]; II < EE; ++II) {
+  unsigned RevIdx = EE - II - 1;
+  Value *DimsLVal = Builder.CreateInBoundsGEP(
+  DimsAddr->getAllocatedType(), DimsAddr,
+  {Builder.getInt64(0), Builder.getInt64(II)});
+  // Offset
+  Value *OffsetLVal = Builder.CreateStructGEP(DimTy, DimsLVal, OffsetFD);
+  Builder.CreateAlignedStore(
+  NonContigInfo.Offsets[L][RevIdx], OffsetLVal,
+  M.getDataLayout().getPrefTypeAlign(OffsetLVal->getType()));
+  // Count
+  Value *CountLVal = Builder.CreateStructGEP(DimTy, DimsLVal, CountFD);
+  Builder.CreateAlignedStore(
+  NonContigInfo.Counts[L][RevIdx], CountLVal,
+  M.getDataLayout().getPrefTypeAlign(CountLVal->getType()));
+  // Stride
+  Value *StrideLVal = Builder.CreateStructGEP(DimTy, DimsLVal, StrideFD);
+  Builder.CreateAlignedStore(
+  NonContigInfo.Strides[L][RevIdx], StrideLVal,
+  M.getDataLayout().getPrefTypeAlign(CountLVal->getType()));
+}
+// args[I] = 
+Builder.restoreIP(CodeGenIP);
+Value *DAddr = Builder.CreatePointerBitCastOrAddrSpaceCast(
+DimsAddr, Builder.getInt8PtrTy());
+Value *P = Builder.CreateConstInBoundsGEP2_32(
+ArrayType::get(Builder.getInt8PtrTy(), Info.NumberOfPtrs),
+Info.RTArgs.PointersArray, 0, I);
+Builder.CreateAlignedStore(
+DAddr, P, M.getDataLayout().getPrefTypeAlign(Builder.getInt8PtrTy()));
+++L;
+  }
+}
+
+void OpenMPIRBuilder::emitOffloadingArrays(
+InsertPointTy AllocaIP, InsertPointTy CodeGenIP, MapInfosTy ,
+TargetDataInfo , bool IsNonContiguous,
+function_ref DeviceAddrCB,
+function_ref CustomMapperCB) {
+
+  // Reset the array information.
+  Info.clearArrayInfo();
+  Info.NumberOfPtrs = CombinedInfo.BasePointers.size();
+
+  if (Info.NumberOfPtrs == 0)
+return;
+
+  Builder.restoreIP(AllocaIP);
+  // Detect if we have any capture size requiring runtime evaluation of the
+  // size so that a constant array could be eventually used.
+  ArrayType *PointerArrayType =
+  ArrayType::get(Builder.getInt8PtrTy(), Info.NumberOfPtrs);
+
+  Info.RTArgs.BasePointersArray = Builder.CreateAlloca(
+  PointerArrayType, /* ArraySize = */ nullptr, ".offload_baseptrs");
+
+  Info.RTArgs.PointersArray = Builder.CreateAlloca(
+  PointerArrayType, /* ArraySize = */ nullptr, ".offload_ptrs");
+  AllocaInst *MappersArray = Builder.CreateAlloca(
+  PointerArrayType, /* ArraySize = */ nullptr, ".offload_mappers");
+  Info.RTArgs.MappersArray = MappersArray;
+
+  // If we don't have any VLA types or other types that require runtime
+  

[PATCH] D149872: [OpenMP][OMPIRBuilder] Migrate emitOffloadingArrays and EmitNonContiguousDescriptor from Clang

[Akash Banerjee via Phabricator via cfe-commits]

TIFitis updated this revision to Diff 530508.
TIFitis marked 4 inline comments as done.
TIFitis added a comment.

Addressed reviewer comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149872

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -4419,6 +4419,256 @@
 Builder.CreatePointerCast(Info.RTArgs.MappersArray, VoidPtrPtrTy);
 }
 
+void OpenMPIRBuilder::emitNonContiguousDescriptor(InsertPointTy AllocaIP,
+  InsertPointTy CodeGenIP,
+  MapInfosTy ,
+  TargetDataInfo ) {
+  MapInfosTy::StructNonContiguousInfo  =
+  CombinedInfo.NonContigInfo;
+
+  // Build an array of struct descriptor_dim and then assign it to
+  // offload_args.
+  //
+  // struct descriptor_dim {
+  //  uint64_t offset;
+  //  uint64_t count;
+  //  uint64_t stride
+  // };
+  Type *Int64Ty = Builder.getInt64Ty();
+  StructType *DimTy = StructType::create(
+  M.getContext(), ArrayRef({Int64Ty, Int64Ty, Int64Ty}),
+  "struct.descriptor_dim");
+
+  enum { OffsetFD = 0, CountFD, StrideFD };
+  // We need two index variable here since the size of "Dims" is the same as
+  // the size of Components, however, the size of offset, count, and stride is
+  // equal to the size of base declaration that is non-contiguous.
+  for (unsigned I = 0, L = 0, E = NonContigInfo.Dims.size(); I < E; ++I) {
+// Skip emitting ir if dimension size is 1 since it cannot be
+// non-contiguous.
+if (NonContigInfo.Dims[I] == 1)
+  continue;
+Builder.restoreIP(AllocaIP);
+ArrayType *ArrayTy = ArrayType::get(DimTy, NonContigInfo.Dims[I]);
+AllocaInst *DimsAddr =
+Builder.CreateAlloca(ArrayTy, /* ArraySize = */ nullptr, "dims");
+Builder.restoreIP(CodeGenIP);
+for (unsigned II = 0, EE = NonContigInfo.Dims[I]; II < EE; ++II) {
+  unsigned RevIdx = EE - II - 1;
+  Value *DimsLVal = Builder.CreateInBoundsGEP(
+  DimsAddr->getAllocatedType(), DimsAddr,
+  {Builder.getInt64(0), Builder.getInt64(II)});
+  // Offset
+  Value *OffsetLVal = Builder.CreateStructGEP(DimTy, DimsLVal, OffsetFD);
+  Builder.CreateAlignedStore(
+  NonContigInfo.Offsets[L][RevIdx], OffsetLVal,
+  M.getDataLayout().getPrefTypeAlign(OffsetLVal->getType()));
+  // Count
+  Value *CountLVal = Builder.CreateStructGEP(DimTy, DimsLVal, CountFD);
+  Builder.CreateAlignedStore(
+  NonContigInfo.Counts[L][RevIdx], CountLVal,
+  M.getDataLayout().getPrefTypeAlign(CountLVal->getType()));
+  // Stride
+  Value *StrideLVal = Builder.CreateStructGEP(DimTy, DimsLVal, StrideFD);
+  Builder.CreateAlignedStore(
+  NonContigInfo.Strides[L][RevIdx], StrideLVal,
+  M.getDataLayout().getPrefTypeAlign(CountLVal->getType()));
+}
+// args[I] = 
+Builder.restoreIP(CodeGenIP);
+Value *DAddr = Builder.CreatePointerBitCastOrAddrSpaceCast(
+DimsAddr, Builder.getInt8PtrTy());
+Value *P = Builder.CreateConstInBoundsGEP2_32(
+ArrayType::get(Builder.getInt8PtrTy(), Info.NumberOfPtrs),
+Info.RTArgs.PointersArray, 0, I);
+Builder.CreateAlignedStore(
+DAddr, P, M.getDataLayout().getPrefTypeAlign(Builder.getInt8PtrTy()));
+++L;
+  }
+}
+
+void OpenMPIRBuilder::emitOffloadingArrays(
+InsertPointTy AllocaIP, InsertPointTy CodeGenIP, MapInfosTy ,
+TargetDataInfo , bool IsNonContiguous,
+function_ref DeviceAddrCB,
+function_ref CustomMapperCB) {
+
+  // Reset the array information.
+  Info.clearArrayInfo();
+  Info.NumberOfPtrs = CombinedInfo.BasePointers.size();
+
+  if (Info.NumberOfPtrs == 0)
+return;
+
+  Builder.restoreIP(AllocaIP);
+  // Detect if we have any capture size requiring runtime evaluation of the
+  // size so that a constant array could be eventually used.
+  ArrayType *PointerArrayType =
+  ArrayType::get(Builder.getInt8PtrTy(), Info.NumberOfPtrs);
+
+  Info.RTArgs.BasePointersArray = Builder.CreateAlloca(
+  PointerArrayType, /* ArraySize = */ nullptr, ".offload_baseptrs");
+
+  Info.RTArgs.PointersArray = Builder.CreateAlloca(
+  PointerArrayType, /* ArraySize = */ nullptr, ".offload_ptrs");
+  AllocaInst *MappersArray = Builder.CreateAlloca(
+  PointerArrayType, /* ArraySize = */ nullptr, ".offload_mappers");
+  Info.RTArgs.MappersArray = MappersArray;
+
+  // If we don't have any VLA types or other types that require runtime
+  // evaluation, we can use a constant array for the map sizes, otherwise we
+  // need to fill up 

[PATCH] D149872: [OpenMP][OMPIRBuilder] Migrate emitOffloadingArrays and EmitNonContiguousDescriptor from Clang

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

If this passes all our tests, it looks fine. A few nits below, try to address 
if possible.




Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4427
+  M.getContext(), ArrayRef({Int64Ty, Int64Ty, Int64Ty}),
+  "struct.descriptor_dim");
+

If it helps, you can define the struct type in OMPKind.td, I think that's the 
name.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4487
+
+  if (Info.NumberOfPtrs) {
+Builder.restoreIP(AllocaIP);

Can we do an early exit here?



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4562
+Info.RTArgs.SizesArray = SizesArrayGbl;
+  }
+  Builder.restoreIP(CodeGenIP);

Generally try to have the "short" branch first, especially one liners.
People have forgotten the condition when they get here.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4618
+  if (Info.requiresDevicePointerInfo()) {
+assert(DeviceAddrCB);
+DeviceAddrCB(I, BP, BPVal);

Add a message to all asserts, please.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149872

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149872: [OpenMP][OMPIRBuilder] Migrate emitOffloadingArrays and EmitNonContiguousDescriptor from Clang

[Akash Banerjee via Phabricator via cfe-commits]

TIFitis added a comment.

Ping for review :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149872

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149872: [OpenMP][OMPIRBuilder] Migrate emitOffloadingArrays and EmitNonContiguousDescriptor from Clang

[Akash Banerjee via Phabricator via cfe-commits]

TIFitis updated this revision to Diff 523340.
TIFitis added a comment.

Rebasing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149872

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -4406,6 +4406,260 @@
 Builder.CreatePointerCast(Info.RTArgs.MappersArray, VoidPtrPtrTy);
 }
 
+void OpenMPIRBuilder::emitNonContiguousDescriptor(InsertPointTy AllocaIP,
+  InsertPointTy CodeGenIP,
+  MapInfosTy ,
+  TargetDataInfo ) {
+  MapInfosTy::StructNonContiguousInfo  =
+  CombinedInfo.NonContigInfo;
+
+  // Build an array of struct descriptor_dim and then assign it to
+  // offload_args.
+  //
+  // struct descriptor_dim {
+  //  uint64_t offset;
+  //  uint64_t count;
+  //  uint64_t stride
+  // };
+  Type *Int64Ty = Builder.getInt64Ty();
+  StructType *DimTy = StructType::create(
+  M.getContext(), ArrayRef({Int64Ty, Int64Ty, Int64Ty}),
+  "struct.descriptor_dim");
+
+  enum { OffsetFD = 0, CountFD, StrideFD };
+  // We need two index variable here since the size of "Dims" is the same as
+  // the size of Components, however, the size of offset, count, and stride is
+  // equal to the size of base declaration that is non-contiguous.
+  for (unsigned I = 0, L = 0, E = NonContigInfo.Dims.size(); I < E; ++I) {
+// Skip emitting ir if dimension size is 1 since it cannot be
+// non-contiguous.
+if (NonContigInfo.Dims[I] == 1)
+  continue;
+Builder.restoreIP(AllocaIP);
+ArrayType *ArrayTy = ArrayType::get(DimTy, NonContigInfo.Dims[I]);
+AllocaInst *DimsAddr =
+Builder.CreateAlloca(ArrayTy, /* ArraySize = */ nullptr, "dims");
+Builder.restoreIP(CodeGenIP);
+for (unsigned II = 0, EE = NonContigInfo.Dims[I]; II < EE; ++II) {
+  unsigned RevIdx = EE - II - 1;
+  Value *DimsLVal = Builder.CreateInBoundsGEP(
+  DimsAddr->getAllocatedType(), DimsAddr,
+  {Builder.getInt64(0), Builder.getInt64(II)});
+  // Offset
+  Value *OffsetLVal = Builder.CreateStructGEP(DimTy, DimsLVal, OffsetFD);
+  Builder.CreateAlignedStore(
+  NonContigInfo.Offsets[L][RevIdx], OffsetLVal,
+  M.getDataLayout().getPrefTypeAlign(OffsetLVal->getType()));
+  // Count
+  Value *CountLVal = Builder.CreateStructGEP(DimTy, DimsLVal, CountFD);
+  Builder.CreateAlignedStore(
+  NonContigInfo.Counts[L][RevIdx], CountLVal,
+  M.getDataLayout().getPrefTypeAlign(CountLVal->getType()));
+  // Stride
+  Value *StrideLVal = Builder.CreateStructGEP(DimTy, DimsLVal, StrideFD);
+  Builder.CreateAlignedStore(
+  NonContigInfo.Strides[L][RevIdx], StrideLVal,
+  M.getDataLayout().getPrefTypeAlign(CountLVal->getType()));
+}
+// args[I] = 
+Builder.restoreIP(CodeGenIP);
+Value *DAddr = Builder.CreatePointerBitCastOrAddrSpaceCast(
+DimsAddr, Builder.getInt8PtrTy());
+Value *P = Builder.CreateConstInBoundsGEP2_32(
+ArrayType::get(Builder.getInt8PtrTy(), Info.NumberOfPtrs),
+Info.RTArgs.PointersArray, 0, I);
+Builder.CreateAlignedStore(
+DAddr, P, M.getDataLayout().getPrefTypeAlign(Builder.getInt8PtrTy()));
+++L;
+  }
+}
+
+void OpenMPIRBuilder::emitOffloadingArrays(
+InsertPointTy AllocaIP, InsertPointTy CodeGenIP, MapInfosTy ,
+TargetDataInfo , bool IsNonContiguous,
+function_ref DeviceAddrCB,
+function_ref CustomMapperCB) {
+
+  // Reset the array information.
+  Info.clearArrayInfo();
+  Info.NumberOfPtrs = CombinedInfo.BasePointers.size();
+
+  if (Info.NumberOfPtrs) {
+Builder.restoreIP(AllocaIP);
+// Detect if we have any capture size requiring runtime evaluation of the
+// size so that a constant array could be eventually used.
+ArrayType *PointerArrayType =
+ArrayType::get(Builder.getInt8PtrTy(), Info.NumberOfPtrs);
+
+Info.RTArgs.BasePointersArray = Builder.CreateAlloca(
+PointerArrayType, /* ArraySize = */ nullptr, ".offload_baseptrs");
+
+Info.RTArgs.PointersArray = Builder.CreateAlloca(
+PointerArrayType, /* ArraySize = */ nullptr, ".offload_ptrs");
+AllocaInst *MappersArray = Builder.CreateAlloca(
+PointerArrayType, /* ArraySize = */ nullptr, ".offload_mappers");
+Info.RTArgs.MappersArray = MappersArray;
+
+// If we don't have any VLA types or other types that require runtime
+// evaluation, we can use a constant array for the map sizes, otherwise we
+// need to fill up the arrays as we do for the pointers.
+Type 

[PATCH] D149872: [OpenMP][OMPIRBuilder] Migrate emitOffloadingArrays and EmitNonContiguousDescriptor from Clang

[Akash Banerjee via Phabricator via cfe-commits]

TIFitis marked 3 inline comments as done.
TIFitis added a comment.

Ping for review :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149872

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149872: [OpenMP][OMPIRBuilder] Migrate emitOffloadingArrays and EmitNonContiguousDescriptor from Clang

[Akash Banerjee via Phabricator via cfe-commits]

TIFitis updated this revision to Diff 520730.
TIFitis added a comment.

Addressed reviewer comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149872

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -4368,6 +4368,260 @@
 Builder.CreatePointerCast(Info.RTArgs.MappersArray, VoidPtrPtrTy);
 }
 
+void OpenMPIRBuilder::emitNonContiguousDescriptor(InsertPointTy AllocaIP,
+  InsertPointTy CodeGenIP,
+  MapInfosTy ,
+  TargetDataInfo ) {
+  MapInfosTy::StructNonContiguousInfo  =
+  CombinedInfo.NonContigInfo;
+
+  // Build an array of struct descriptor_dim and then assign it to
+  // offload_args.
+  //
+  // struct descriptor_dim {
+  //  uint64_t offset;
+  //  uint64_t count;
+  //  uint64_t stride
+  // };
+  Type *Int64Ty = Builder.getInt64Ty();
+  StructType *DimTy = StructType::create(
+  M.getContext(), ArrayRef({Int64Ty, Int64Ty, Int64Ty}),
+  "struct.descriptor_dim");
+
+  enum { OffsetFD = 0, CountFD, StrideFD };
+  // We need two index variable here since the size of "Dims" is the same as
+  // the size of Components, however, the size of offset, count, and stride is
+  // equal to the size of base declaration that is non-contiguous.
+  for (unsigned I = 0, L = 0, E = NonContigInfo.Dims.size(); I < E; ++I) {
+// Skip emitting ir if dimension size is 1 since it cannot be
+// non-contiguous.
+if (NonContigInfo.Dims[I] == 1)
+  continue;
+Builder.restoreIP(AllocaIP);
+ArrayType *ArrayTy = ArrayType::get(DimTy, NonContigInfo.Dims[I]);
+AllocaInst *DimsAddr =
+Builder.CreateAlloca(ArrayTy, /* ArraySize = */ nullptr, "dims");
+Builder.restoreIP(CodeGenIP);
+for (unsigned II = 0, EE = NonContigInfo.Dims[I]; II < EE; ++II) {
+  unsigned RevIdx = EE - II - 1;
+  Value *DimsLVal = Builder.CreateInBoundsGEP(
+  DimsAddr->getAllocatedType(), DimsAddr,
+  {Builder.getInt64(0), Builder.getInt64(II)});
+  // Offset
+  Value *OffsetLVal = Builder.CreateStructGEP(DimTy, DimsLVal, OffsetFD);
+  Builder.CreateAlignedStore(
+  NonContigInfo.Offsets[L][RevIdx], OffsetLVal,
+  M.getDataLayout().getPrefTypeAlign(OffsetLVal->getType()));
+  // Count
+  Value *CountLVal = Builder.CreateStructGEP(DimTy, DimsLVal, CountFD);
+  Builder.CreateAlignedStore(
+  NonContigInfo.Counts[L][RevIdx], CountLVal,
+  M.getDataLayout().getPrefTypeAlign(CountLVal->getType()));
+  // Stride
+  Value *StrideLVal = Builder.CreateStructGEP(DimTy, DimsLVal, StrideFD);
+  Builder.CreateAlignedStore(
+  NonContigInfo.Strides[L][RevIdx], StrideLVal,
+  M.getDataLayout().getPrefTypeAlign(CountLVal->getType()));
+}
+// args[I] = 
+Builder.restoreIP(CodeGenIP);
+Value *DAddr = Builder.CreatePointerBitCastOrAddrSpaceCast(
+DimsAddr, Builder.getInt8PtrTy());
+Value *P = Builder.CreateConstInBoundsGEP2_32(
+ArrayType::get(Builder.getInt8PtrTy(), Info.NumberOfPtrs),
+Info.RTArgs.PointersArray, 0, I);
+Builder.CreateAlignedStore(
+DAddr, P, M.getDataLayout().getPrefTypeAlign(Builder.getInt8PtrTy()));
+++L;
+  }
+}
+
+void OpenMPIRBuilder::emitOffloadingArrays(
+InsertPointTy AllocaIP, InsertPointTy CodeGenIP, MapInfosTy ,
+TargetDataInfo , bool IsNonContiguous,
+function_ref DeviceAddrCB,
+function_ref CustomMapperCB) {
+
+  // Reset the array information.
+  Info.clearArrayInfo();
+  Info.NumberOfPtrs = CombinedInfo.BasePointers.size();
+
+  if (Info.NumberOfPtrs) {
+Builder.restoreIP(AllocaIP);
+// Detect if we have any capture size requiring runtime evaluation of the
+// size so that a constant array could be eventually used.
+ArrayType *PointerArrayType =
+ArrayType::get(Builder.getInt8PtrTy(), Info.NumberOfPtrs);
+
+Info.RTArgs.BasePointersArray = Builder.CreateAlloca(
+PointerArrayType, /* ArraySize = */ nullptr, ".offload_baseptrs");
+
+Info.RTArgs.PointersArray = Builder.CreateAlloca(
+PointerArrayType, /* ArraySize = */ nullptr, ".offload_ptrs");
+AllocaInst *MappersArray = Builder.CreateAlloca(
+PointerArrayType, /* ArraySize = */ nullptr, ".offload_mappers");
+Info.RTArgs.MappersArray = MappersArray;
+
+// If we don't have any VLA types or other types that require runtime
+// evaluation, we can use a constant array for the map sizes, otherwise we
+// need to fill up the arrays as we do for the 

[PATCH] D149872: [OpenMP][OMPIRBuilder] Migrate emitOffloadingArrays and EmitNonContiguousDescriptor from Clang

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

jsjodin added inline comments.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4376
+function_ref CustomMapperCB) {
+  auto EmitNonContiguousDescriptor = [&]() {
+MapInfosTy::StructNonContiguousInfo  =

I don't think we need a lambda function, it can be a regular method or static 
function?



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4576
+  if (Info.requiresDevicePointerInfo())
+DeviceAddrCB(I, BP, BPVal);
+

Do we need to check if DeviceAddrCB is null?



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4601
+  Value *MFunc = ConstantPointerNull::get(Builder.getInt8PtrTy());
+  if (Value *CustomMFunc = CustomMapperCB(I))
+MFunc = Builder.CreatePointerCast(CustomMFunc, Builder.getInt8PtrTy());

Check if CustomMapperCB is nullptr?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149872

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149872: [OpenMP][OMPIRBuilder] Migrate emitOffloadingArrays and EmitNonContiguousDescriptor from Clang

[Akash Banerjee via Phabricator via cfe-commits]

TIFitis updated this revision to Diff 519822.
TIFitis added a comment.

Changed std::function to llvm::function_ref for the callbacks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149872

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -4368,6 +4368,252 @@
 Builder.CreatePointerCast(Info.RTArgs.MappersArray, VoidPtrPtrTy);
 }
 
+void OpenMPIRBuilder::emitOffloadingArrays(
+InsertPointTy AllocaIP, InsertPointTy CodeGenIP, MapInfosTy ,
+TargetDataInfo , bool IsNonContiguous,
+function_ref DeviceAddrCB,
+function_ref CustomMapperCB) {
+  auto EmitNonContiguousDescriptor = [&]() {
+MapInfosTy::StructNonContiguousInfo  =
+CombinedInfo.NonContigInfo;
+
+// Build an array of struct descriptor_dim and then assign it to
+// offload_args.
+//
+// struct descriptor_dim {
+//  uint64_t offset;
+//  uint64_t count;
+//  uint64_t stride
+// };
+Type *Int64Ty = Builder.getInt64Ty();
+StructType *DimTy = StructType::create(
+M.getContext(), ArrayRef({Int64Ty, Int64Ty, Int64Ty}),
+"struct.descriptor_dim");
+
+enum { OffsetFD = 0, CountFD, StrideFD };
+// We need two index variable here since the size of "Dims" is the same as
+// the size of Components, however, the size of offset, count, and stride is
+// equal to the size of base declaration that is non-contiguous.
+for (unsigned I = 0, L = 0, E = NonContigInfo.Dims.size(); I < E; ++I) {
+  // Skip emitting ir if dimension size is 1 since it cannot be
+  // non-contiguous.
+  if (NonContigInfo.Dims[I] == 1)
+continue;
+  Builder.restoreIP(AllocaIP);
+  ArrayType *ArrayTy = ArrayType::get(DimTy, NonContigInfo.Dims[I]);
+  AllocaInst *DimsAddr =
+  Builder.CreateAlloca(ArrayTy, /* ArraySize = */ nullptr, "dims");
+  Builder.restoreIP(CodeGenIP);
+  for (unsigned II = 0, EE = NonContigInfo.Dims[I]; II < EE; ++II) {
+unsigned RevIdx = EE - II - 1;
+Value *DimsLVal = Builder.CreateInBoundsGEP(
+DimsAddr->getAllocatedType(), DimsAddr,
+{Builder.getInt64(0), Builder.getInt64(II)});
+// Offset
+Value *OffsetLVal = Builder.CreateStructGEP(DimTy, DimsLVal, OffsetFD);
+Builder.CreateAlignedStore(
+NonContigInfo.Offsets[L][RevIdx], OffsetLVal,
+M.getDataLayout().getPrefTypeAlign(OffsetLVal->getType()));
+// Count
+Value *CountLVal = Builder.CreateStructGEP(DimTy, DimsLVal, CountFD);
+Builder.CreateAlignedStore(
+NonContigInfo.Counts[L][RevIdx], CountLVal,
+M.getDataLayout().getPrefTypeAlign(CountLVal->getType()));
+// Stride
+Value *StrideLVal = Builder.CreateStructGEP(DimTy, DimsLVal, StrideFD);
+Builder.CreateAlignedStore(
+NonContigInfo.Strides[L][RevIdx], StrideLVal,
+M.getDataLayout().getPrefTypeAlign(CountLVal->getType()));
+  }
+  // args[I] = 
+  Builder.restoreIP(CodeGenIP);
+  Value *DAddr = Builder.CreatePointerBitCastOrAddrSpaceCast(
+  DimsAddr, Builder.getInt8PtrTy());
+  Value *P = Builder.CreateConstInBoundsGEP2_32(
+  ArrayType::get(Builder.getInt8PtrTy(), Info.NumberOfPtrs),
+  Info.RTArgs.PointersArray, 0, I);
+  Builder.CreateAlignedStore(
+  DAddr, P, M.getDataLayout().getPrefTypeAlign(Builder.getInt8PtrTy()));
+  ++L;
+}
+  };
+
+  // Reset the array information.
+  Info.clearArrayInfo();
+  Info.NumberOfPtrs = CombinedInfo.BasePointers.size();
+
+  if (Info.NumberOfPtrs) {
+Builder.restoreIP(AllocaIP);
+// Detect if we have any capture size requiring runtime evaluation of the
+// size so that a constant array could be eventually used.
+ArrayType *PointerArrayType =
+ArrayType::get(Builder.getInt8PtrTy(), Info.NumberOfPtrs);
+
+Info.RTArgs.BasePointersArray = Builder.CreateAlloca(
+PointerArrayType, /* ArraySize = */ nullptr, ".offload_baseptrs");
+
+Info.RTArgs.PointersArray = Builder.CreateAlloca(
+PointerArrayType, /* ArraySize = */ nullptr, ".offload_ptrs");
+AllocaInst *MappersArray = Builder.CreateAlloca(
+PointerArrayType, /* ArraySize = */ nullptr, ".offload_mappers");
+Info.RTArgs.MappersArray = MappersArray;
+
+// If we don't have any VLA types or other types that require runtime
+// evaluation, we can use a constant array for the map sizes, otherwise we
+// need to fill up the arrays as we do for the pointers.
+Type *Int64Ty = Builder.getInt64Ty();
+SmallVector ConstSizes(
+  

[PATCH] D149872: [OpenMP][OMPIRBuilder] Migrate emitOffloadingArrays and EmitNonContiguousDescriptor from Clang

[Akash Banerjee via Phabricator via cfe-commits]

TIFitis created this revision.
TIFitis added reviewers: jdoerfert, jsjodin.
Herald added subscribers: sunshaoce, guansong, hiraditya, yaxunl.
Herald added a project: All.
TIFitis requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, jplehr, sstefan1.
Herald added projects: clang, LLVM.

This patch migrates the emitOffloadingArrays and EmitNonContiguousDescriptor 
functions from Clang codegen to OpenMPIRBuilder.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149872

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -4368,6 +4368,252 @@
 Builder.CreatePointerCast(Info.RTArgs.MappersArray, VoidPtrPtrTy);
 }
 
+void OpenMPIRBuilder::emitOffloadingArrays(
+InsertPointTy AllocaIP, InsertPointTy CodeGenIP, MapInfosTy ,
+TargetDataInfo , bool IsNonContiguous,
+std::function DeviceAddrCB,
+std::function CustomMapperCB) {
+  auto EmitNonContiguousDescriptor = [&]() {
+MapInfosTy::StructNonContiguousInfo  =
+CombinedInfo.NonContigInfo;
+
+// Build an array of struct descriptor_dim and then assign it to
+// offload_args.
+//
+// struct descriptor_dim {
+//  uint64_t offset;
+//  uint64_t count;
+//  uint64_t stride
+// };
+Type *Int64Ty = Builder.getInt64Ty();
+StructType *DimTy = StructType::create(
+M.getContext(), ArrayRef({Int64Ty, Int64Ty, Int64Ty}),
+"struct.descriptor_dim");
+
+enum { OffsetFD = 0, CountFD, StrideFD };
+// We need two index variable here since the size of "Dims" is the same as
+// the size of Components, however, the size of offset, count, and stride is
+// equal to the size of base declaration that is non-contiguous.
+for (unsigned I = 0, L = 0, E = NonContigInfo.Dims.size(); I < E; ++I) {
+  // Skip emitting ir if dimension size is 1 since it cannot be
+  // non-contiguous.
+  if (NonContigInfo.Dims[I] == 1)
+continue;
+  Builder.restoreIP(AllocaIP);
+  ArrayType *ArrayTy = ArrayType::get(DimTy, NonContigInfo.Dims[I]);
+  AllocaInst *DimsAddr =
+  Builder.CreateAlloca(ArrayTy, /* ArraySize = */ nullptr, "dims");
+  Builder.restoreIP(CodeGenIP);
+  for (unsigned II = 0, EE = NonContigInfo.Dims[I]; II < EE; ++II) {
+unsigned RevIdx = EE - II - 1;
+Value *DimsLVal = Builder.CreateInBoundsGEP(
+DimsAddr->getAllocatedType(), DimsAddr,
+{Builder.getInt64(0), Builder.getInt64(II)});
+// Offset
+Value *OffsetLVal = Builder.CreateStructGEP(DimTy, DimsLVal, OffsetFD);
+Builder.CreateAlignedStore(
+NonContigInfo.Offsets[L][RevIdx], OffsetLVal,
+M.getDataLayout().getPrefTypeAlign(OffsetLVal->getType()));
+// Count
+Value *CountLVal = Builder.CreateStructGEP(DimTy, DimsLVal, CountFD);
+Builder.CreateAlignedStore(
+NonContigInfo.Counts[L][RevIdx], CountLVal,
+M.getDataLayout().getPrefTypeAlign(CountLVal->getType()));
+// Stride
+Value *StrideLVal = Builder.CreateStructGEP(DimTy, DimsLVal, StrideFD);
+Builder.CreateAlignedStore(
+NonContigInfo.Strides[L][RevIdx], StrideLVal,
+M.getDataLayout().getPrefTypeAlign(CountLVal->getType()));
+  }
+  // args[I] = 
+  Builder.restoreIP(CodeGenIP);
+  Value *DAddr = Builder.CreatePointerBitCastOrAddrSpaceCast(
+  DimsAddr, Builder.getInt8PtrTy());
+  Value *P = Builder.CreateConstInBoundsGEP2_32(
+  ArrayType::get(Builder.getInt8PtrTy(), Info.NumberOfPtrs),
+  Info.RTArgs.PointersArray, 0, I);
+  Builder.CreateAlignedStore(
+  DAddr, P, M.getDataLayout().getPrefTypeAlign(Builder.getInt8PtrTy()));
+  ++L;
+}
+  };
+
+  // Reset the array information.
+  Info.clearArrayInfo();
+  Info.NumberOfPtrs = CombinedInfo.BasePointers.size();
+
+  if (Info.NumberOfPtrs) {
+Builder.restoreIP(AllocaIP);
+// Detect if we have any capture size requiring runtime evaluation of the
+// size so that a constant array could be eventually used.
+ArrayType *PointerArrayType =
+ArrayType::get(Builder.getInt8PtrTy(), Info.NumberOfPtrs);
+
+Info.RTArgs.BasePointersArray = Builder.CreateAlloca(
+PointerArrayType, /* ArraySize = */ nullptr, ".offload_baseptrs");
+
+Info.RTArgs.PointersArray = Builder.CreateAlloca(
+PointerArrayType, /* ArraySize = */ nullptr, ".offload_ptrs");
+AllocaInst *MappersArray = Builder.CreateAlloca(
+PointerArrayType, /* ArraySize = */ nullptr, ".offload_mappers");
+Info.RTArgs.MappersArray = MappersArray;
+
+// If we don't have any VLA types or