dhruvachak added a comment. In D102107#3633705 <https://reviews.llvm.org/D102107#3633705>, @jdoerfert wrote:
> Also, make sure to remove all deviceRTL files and probably reset the > autogenerated tests to upstream (and re-generate) before you merge (or > reupload). > > In D102107#3633678 <https://reviews.llvm.org/D102107#3633678>, @dhruvachak > wrote: > >> I rebased and resolved conflicts just now and got the compiler built. I did >> not update the tests, hence not updating this review. I see the following >> outstanding issues: >> >> (1) make check-libomptarget produces a bunch of failures with the following >> compile-time assertion. So my rebased patch is not interacting correctly >> with opaque pointers. It is the same assertion for all the failures. >> llvm-project/llvm/include/llvm/IR/Type.h:384: llvm::Type* >> llvm::Type::getNonOpaquePointerElementType() const: Assertion >> `NumContainedTys && "Attempting to get element type of opaque pointer"' >> failed. > > See my comment below. I think that's the issue. > >> (2) From earlier investigation a couple of months back, this patch uses >> device alloc and will fail if device allocation is not implemented (e.g. in >> main branch of amdgpu). Most of these failures are seen at -O0, OpenMPOpt is >> able to optimize them away at higher opt levels. Are we ok with these >> failures at -O0? > > It used __kmpc_alloc_shared, which should in theory work with O0 (also for > AMDGPU) but in practice might not, especially if it has to fallback to > malloc. We are working on malloc support right now. This should not stop us. > No reasonable code runs with O0 (on AMDGPU) right now. > >> (3) There were a few issues found regarding SPDMization, NoCaptureAttrs, >> alignment that should be applied to this patch. I have those changes on a >> local branch. > > Apply them, I can look over everything again. Yes, I will apply the changes, refresh the tests, and re-upload. ================ Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:1540 + + llvm::Type *PtrElemTy = AggregateV->getType()->getPointerElementType(); + auto &DL = CGM.getDataLayout(); ---------------- jdoerfert wrote: > This doesn't work anymore with opaque pointers, IIRC. We should remember the > type and pass to this place. Thanks. Changing this fixed the assertions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102107/new/ https://reviews.llvm.org/D102107 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
