[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<Type *>({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