ABataev added inline comments.
================ Comment at: clang/include/clang/Basic/OpenMPKinds.h:23-24 enum OpenMPDirectiveKind { -#define OPENMP_DIRECTIVE(Name) \ - OMPD_##Name, #define OPENMP_DIRECTIVE_EXT(Name, Str) \ ---------------- Better to make in a separate NFC patch. ================ Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:3483 + auto *OMPRegionInfo = dyn_cast_or_null<CGOpenMPRegionInfo>(CGF.CapturedStmtInfo); + if (!EmitChecks || ForceSimpleCall || !OMPRegionInfo || + !OMPRegionInfo->hasCancel()) { ---------------- I would add an option, something like `-fopenmp-experimental` for all changes with OpenMPBulder unless it at least matches in functionality/performance with the existing solution. ================ Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:3486 + CGF.CGM.getOpenMPIRBuilder().emitOMPBarrier( + {CGF.Builder.saveIP()}, llvm::OpenMPIRBuilder::DirektiveKind(Kind), + EmitChecks); ---------------- `DirectiveKind` ================ Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:3493 return; + // Build call __kmpc_cancel_barrier(loc, thread_id); ---------------- Better to remove all these formatting changes from the patch. ================ Comment at: llvm/include/llvm/IR/OpenMPIRBuilder.h:33 + /// IDs for all OpenMP directives. + enum DirektiveKind { +#define OMP_DIRECTIVE(Enum, ...) Enum, ---------------- 1. `DirectiveKind` 2. Is it possible to merge these 2 types in LLVM and in clang into 1? ================ Comment at: llvm/lib/IR/OpenMPIRBuilder.cpp:108 + DefaultIdent->setUnnamedAddr(GlobalValue::UnnamedAddr::Global); + DefaultIdent->setAlignment(Align(8)); + } ---------------- Is this correct for 32-bit platforms? Maybe rely on the frontend when creating structures and all other required feature things? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69785/new/ https://reviews.llvm.org/D69785 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits