tianshilei1992 created this revision. tianshilei1992 added reviewers: jdoerfert, ABataev. Herald added subscribers: guansong, yaxunl. Herald added a project: All. tianshilei1992 requested review of this revision. Herald added subscribers: openmp-commits, cfe-commits, sstefan1. Herald added projects: clang, OpenMP.
In Clang, in order to determine the type of `omp_allocator_handle_t`, Clang checks the type of those predefined allocators. The first one it checks is `omp_null_allocator`. If the language is C, and the system is 64-bit, what Clang gets is a `int`, instead of an enum of size 8, given the fact how we define `omp_allocator_handle_t` in `omp.h`. If the allocator is captured by a region, let's say a parallel region, the allocator will be privatized. Because Clang deems `omp_allocator_handle_t` as an `int`, it will first cast the value returned by the runtime library (for `libomp` it is a `void *`) to `int`, and then in the outlined function, it casts back to `omp_allocator_handle_t`. This two casts completely shaves the first 32-bit of the pointer value returned from `libomp`, and when the private "new" pointer is fed to another runtime function `__kmpc_allocate()`, it causes segment fault. That is the root cause of PR54082. I have no idea why `-fno-pic` could hide this bug. In this patch, we detect `omp_allocator_handle_t` using roughly the same method as `omp_event_handle_t`, by looking it up into the identifier table. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D142297 Files: clang/lib/Sema/SemaOpenMP.cpp clang/test/OpenMP/target_uses_allocators.c openmp/runtime/test/parallel/bug54082.c
Index: openmp/runtime/test/parallel/bug54082.c =================================================================== --- /dev/null +++ openmp/runtime/test/parallel/bug54082.c @@ -0,0 +1,54 @@ +// This test is adapted from test_parallel_for_allocate.c in SOLLVE V&V. +// https://github.com/SOLLVE/sollve_vv/blob/master/tests/5.0/parallel_for/test_parallel_for_allocate.c +// RUN: %libomp-compile-and-run +#include <omp.h> + +#include <assert.h> +#include <stdlib.h> + +#define N 1024 + +int main(int argc, char *argv[]) { + int errors = 0; + int *x; + int result[N][N]; + int successful_alloc = 0; + + omp_memspace_handle_t x_memspace = omp_default_mem_space; + omp_alloctrait_t x_traits[1] = {omp_atk_alignment, 64}; + omp_allocator_handle_t x_alloc = omp_init_allocator(x_memspace, 1, x_traits); + + for (int i = 0; i < N; i++) { + for (int j = 0; j < N; j++) { + result[i][j] = -1; + } + } + +#pragma omp parallel for allocate(x_alloc: x) private(x) shared(result) + for (int i = 0; i < N; i++) { + x = (int *)malloc(N * sizeof(int)); + if (x != NULL) { +#pragma omp simd simdlen(16) aligned(x : 64) + for (int j = 0; j < N; j++) { + x[j] = j * i; + } + for (int j = 0; j < N; j++) { + result[i][j] = x[j]; + } + free(x); + successful_alloc++; + } + } + + errors += successful_alloc < 1; + + for (int i = 0; i < N; i++) { + for (int j = 0; j < N; j++) { + errors += result[i][j] != i * j; + } + } + + omp_destroy_allocator(x_alloc); + + return errors; +} Index: clang/test/OpenMP/target_uses_allocators.c =================================================================== --- clang/test/OpenMP/target_uses_allocators.c +++ clang/test/OpenMP/target_uses_allocators.c @@ -6,7 +6,7 @@ #ifndef HEADER #define HEADER -enum omp_allocator_handle_t { +typedef enum omp_allocator_handle_t { omp_null_allocator = 0, omp_default_mem_alloc = 1, omp_large_cap_mem_alloc = 2, @@ -17,7 +17,7 @@ omp_pteam_mem_alloc = 7, omp_thread_mem_alloc = 8, KMP_ALLOCATOR_MAX_HANDLE = __UINTPTR_MAX__ -}; +} omp_allocator_handle_t; // CHECK: define {{.*}}[[FIE:@.+]]() void fie(void) { @@ -105,4 +105,4 @@ // CHECK-NEXT: %.x..void.addr = call ptr @__kmpc_alloc(i32 %[[#R0]], i64 4, ptr inttoptr (i64 8 to ptr)) // CHECK-NEXT: %[[#R1:]] = load i32, ptr %x.addr, align 4 // CHECK-NEXT: store i32 %[[#R1]], ptr %.x..void.addr, align 4 -// CHECK-NEXT: call void @__kmpc_free(i32 %[[#R0]], ptr %.x..void.addr, ptr inttoptr (i64 8 to ptr)) \ No newline at end of file +// CHECK-NEXT: call void @__kmpc_free(i32 %[[#R0]], ptr %.x..void.addr, ptr inttoptr (i64 8 to ptr)) Index: clang/lib/Sema/SemaOpenMP.cpp =================================================================== --- clang/lib/Sema/SemaOpenMP.cpp +++ clang/lib/Sema/SemaOpenMP.cpp @@ -3280,13 +3280,15 @@ Allocator->containsUnexpandedParameterPack()) return OMPAllocateDeclAttr::OMPUserDefinedMemAlloc; auto AllocatorKindRes = OMPAllocateDeclAttr::OMPUserDefinedMemAlloc; + llvm::FoldingSetNodeID AEId; const Expr *AE = Allocator->IgnoreParenImpCasts(); + AE->IgnoreImpCasts()->Profile(AEId, S.getASTContext(), /*Canonical=*/true); for (int I = 0; I < OMPAllocateDeclAttr::OMPUserDefinedMemAlloc; ++I) { auto AllocatorKind = static_cast<OMPAllocateDeclAttr::AllocatorTypeTy>(I); const Expr *DefAllocator = Stack->getAllocator(AllocatorKind); - llvm::FoldingSetNodeID AEId, DAEId; - AE->Profile(AEId, S.getASTContext(), /*Canonical=*/true); - DefAllocator->Profile(DAEId, S.getASTContext(), /*Canonical=*/true); + llvm::FoldingSetNodeID DAEId; + DefAllocator->IgnoreImpCasts()->Profile(DAEId, S.getASTContext(), + /*Canonical=*/true); if (AEId == DAEId) { AllocatorKindRes = AllocatorKind; break; @@ -16473,10 +16475,22 @@ /// Tries to find omp_allocator_handle_t type. static bool findOMPAllocatorHandleT(Sema &S, SourceLocation Loc, DSAStackTy *Stack) { - QualType OMPAllocatorHandleT = Stack->getOMPAllocatorHandleT(); - if (!OMPAllocatorHandleT.isNull()) + if (!Stack->getOMPAllocatorHandleT().isNull()) return true; - // Build the predefined allocator expressions. + + // Set the allocator handle type. + IdentifierInfo *II = &S.PP.getIdentifierTable().get("omp_allocator_handle_t"); + ParsedType PT = S.getTypeName(*II, Loc, S.getCurScope()); + if (!PT.getAsOpaquePtr() || PT.get().isNull()) { + S.Diag(Loc, diag::err_omp_implied_type_not_found) + << "omp_allocator_handle_t"; + return false; + } + QualType AllocatorHandleEnumTy = PT.get(); + AllocatorHandleEnumTy.addConst(); + Stack->setOMPAllocatorHandleT(AllocatorHandleEnumTy); + + // Fill the predefined allocator map. bool ErrorFound = false; for (int I = 0; I < OMPAllocateDeclAttr::OMPUserDefinedMemAlloc; ++I) { auto AllocatorKind = static_cast<OMPAllocateDeclAttr::AllocatorTypeTy>(I); @@ -16496,9 +16510,10 @@ ErrorFound = true; break; } - if (OMPAllocatorHandleT.isNull()) - OMPAllocatorHandleT = AllocatorType; - if (!S.getASTContext().hasSameType(OMPAllocatorHandleT, AllocatorType)) { + Res = S.PerformImplicitConversion(Res.get(), AllocatorHandleEnumTy, + Sema::AA_Initializing, + /* AllowExplicit */ true); + if (!Res.isUsable()) { ErrorFound = true; break; } @@ -16509,8 +16524,7 @@ << "omp_allocator_handle_t"; return false; } - OMPAllocatorHandleT.addConst(); - Stack->setOMPAllocatorHandleT(OMPAllocatorHandleT); + return true; } @@ -23632,17 +23646,26 @@ AllocatorExpr = D.Allocator->IgnoreParenImpCasts(); auto *DRE = dyn_cast<DeclRefExpr>(AllocatorExpr); bool IsPredefinedAllocator = false; - if (DRE) - IsPredefinedAllocator = PredefinedAllocators.count(DRE->getDecl()); - if (!DRE || - !(Context.hasSameUnqualifiedType( - AllocatorExpr->getType(), DSAStack->getOMPAllocatorHandleT()) || - Context.typesAreCompatible(AllocatorExpr->getType(), - DSAStack->getOMPAllocatorHandleT(), - /*CompareUnqualified=*/true)) || - (!IsPredefinedAllocator && - (AllocatorExpr->getType().isConstant(Context) || - !AllocatorExpr->isLValue()))) { + if (DRE) { + OMPAllocateDeclAttr::AllocatorTypeTy AllocatorTy = + getAllocatorKind(*this, DSAStack, AllocatorExpr); + IsPredefinedAllocator = + AllocatorTy != + OMPAllocateDeclAttr::AllocatorTypeTy::OMPUserDefinedMemAlloc; + } + QualType OMPAllocatorHandleT = DSAStack->getOMPAllocatorHandleT(); + QualType AllocatorExprType = AllocatorExpr->getType(); + bool IsTypeCompatible = IsPredefinedAllocator; + IsTypeCompatible = IsTypeCompatible || + Context.hasSameUnqualifiedType(AllocatorExprType, + OMPAllocatorHandleT); + IsTypeCompatible = + IsTypeCompatible || + Context.typesAreCompatible(AllocatorExprType, OMPAllocatorHandleT); + bool IsNonConstantLValue = + !AllocatorExprType.isConstant(Context) && AllocatorExpr->isLValue(); + if (!DRE || !IsTypeCompatible || + (!IsPredefinedAllocator && !IsNonConstantLValue)) { Diag(D.Allocator->getExprLoc(), diag::err_omp_var_expected) << "omp_allocator_handle_t" << (DRE ? 1 : 0) << AllocatorExpr->getType() << D.Allocator->getSourceRange();
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
