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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits