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: cfe-commits, sstefan1.
Herald added a project: clang.
This patch fixes the issue that the globalized variable is not properly
initialized when it is a byval struct function argument.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D129008
Files:
clang/lib/CodeGen/CGDecl.cpp
clang/test/OpenMP/globalization_byval_struct.c
Index: clang/test/OpenMP/globalization_byval_struct.c
===================================================================
--- /dev/null
+++ clang/test/OpenMP/globalization_byval_struct.c
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -no-opaque-pointers -verify -fopenmp -x c -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -no-opaque-pointers -verify -fopenmp -x c -triple nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
+// expected-no-diagnostics
+
+extern int printf(const char *, ...);
+
+struct S {
+ int a;
+ float b;
+};
+
+// CHECK: define{{.*}}void @test(%struct.S* noundef byval(%struct.S) align {{[0-9]+}} [[arg:%[0-9a-zA-Z]+]])
+// CHECK: [[g:%[0-9a-zA-Z]+]] = call align {{[0-9]+}} i8* @__kmpc_alloc_shared
+// CHECK: bitcast i8* [[g]] to %struct.S*
+// CHECK: bitcast %struct.S* [[arg]] to i8**
+// CHECK: call void [[cc:@__copy_constructor[_0-9a-zA-Z]+]]
+// CHECK: void [[cc]]
+void test(struct S s) {
+#pragma omp parallel for
+ for (int i = 0; i < s.a; ++i) {
+ printf("%i : %i : %f\n", i, s.a, s.b);
+ }
+}
+
+void foo() {
+ #pragma omp target teams num_teams(1)
+ {
+ struct S s;
+ s.a = 7;
+ s.b = 11;
+ test(s);
+ }
+}
Index: clang/lib/CodeGen/CGDecl.cpp
===================================================================
--- clang/lib/CodeGen/CGDecl.cpp
+++ clang/lib/CodeGen/CGDecl.cpp
@@ -2471,65 +2471,66 @@
Address DeclPtr = Address::invalid();
Address AllocaPtr = Address::invalid();
bool DoStore = false;
+ bool DoCopy = false;
bool IsScalar = hasScalarEvaluationKind(Ty);
- // If we already have a pointer to the argument, reuse the input pointer.
- if (Arg.isIndirect()) {
- // If we have a prettier pointer type at this point, bitcast to that.
- DeclPtr = Arg.getIndirectAddress();
- DeclPtr = Builder.CreateElementBitCast(DeclPtr, ConvertTypeForMem(Ty),
- D.getName());
- // Indirect argument is in alloca address space, which may be different
- // from the default address space.
- auto AllocaAS = CGM.getASTAllocaAddressSpace();
- auto *V = DeclPtr.getPointer();
- AllocaPtr = DeclPtr;
- auto SrcLangAS = getLangOpts().OpenCL ? LangAS::opencl_private : AllocaAS;
- auto DestLangAS =
- getLangOpts().OpenCL ? LangAS::opencl_private : LangAS::Default;
- if (SrcLangAS != DestLangAS) {
- assert(getContext().getTargetAddressSpace(SrcLangAS) ==
- CGM.getDataLayout().getAllocaAddrSpace());
- auto DestAS = getContext().getTargetAddressSpace(DestLangAS);
- auto *T = DeclPtr.getElementType()->getPointerTo(DestAS);
- DeclPtr = DeclPtr.withPointer(getTargetHooks().performAddrSpaceCast(
- *this, V, SrcLangAS, DestLangAS, T, true));
- }
- // Push a destructor cleanup for this parameter if the ABI requires it.
- // Don't push a cleanup in a thunk for a method that will also emit a
- // cleanup.
- if (Ty->isRecordType() && !CurFuncIsThunk &&
- Ty->castAs<RecordType>()->getDecl()->isParamDestroyedInCallee()) {
- if (QualType::DestructionKind DtorKind =
- D.needsDestruction(getContext())) {
- assert((DtorKind == QualType::DK_cxx_destructor ||
- DtorKind == QualType::DK_nontrivial_c_struct) &&
- "unexpected destructor type");
- pushDestroy(DtorKind, DeclPtr, Ty);
- CalleeDestructedParamCleanups[cast<ParmVarDecl>(&D)] =
- EHStack.stable_begin();
- }
- }
- } else {
- // Check if the parameter address is controlled by OpenMP runtime.
- Address OpenMPLocalAddr =
- getLangOpts().OpenMP
- ? CGM.getOpenMPRuntime().getAddressOfLocalVariable(*this, &D)
- : Address::invalid();
- if (getLangOpts().OpenMP && OpenMPLocalAddr.isValid()) {
- DeclPtr = OpenMPLocalAddr;
+ // We first check if the parameter address is controlled by OpenMP.
+ if (getLangOpts().OpenMP) {
+ DeclPtr = CGM.getOpenMPRuntime().getAddressOfLocalVariable(*this, &D);
+ if (DeclPtr.isValid()) {
AllocaPtr = DeclPtr;
- } else {
- // Otherwise, create a temporary to hold the value.
- DeclPtr = CreateMemTemp(Ty, getContext().getDeclAlign(&D),
- D.getName() + ".addr", &AllocaPtr);
+ DoCopy = true;
}
- DoStore = true;
}
- llvm::Value *ArgVal = (DoStore ? Arg.getDirectValue() : nullptr);
+ // The parameter is not controlled by OpenMP.
+ if (!DeclPtr.isValid()) {
+ // If we already have a pointer to the argument, reuse the input pointer.
+ if (Arg.isIndirect()) {
+ // If we have a prettier pointer type at this point, bitcast to that.
+ DeclPtr = Arg.getIndirectAddress();
+ DeclPtr = Builder.CreateElementBitCast(DeclPtr, ConvertTypeForMem(Ty),
+ D.getName());
+ // Indirect argument is in alloca address space, which may be different
+ // from the default address space.
+ auto AllocaAS = CGM.getASTAllocaAddressSpace();
+ auto *V = DeclPtr.getPointer();
+ AllocaPtr = DeclPtr;
+ auto SrcLangAS = getLangOpts().OpenCL ? LangAS::opencl_private : AllocaAS;
+ auto DestLangAS =
+ getLangOpts().OpenCL ? LangAS::opencl_private : LangAS::Default;
+ if (SrcLangAS != DestLangAS) {
+ assert(getContext().getTargetAddressSpace(SrcLangAS) ==
+ CGM.getDataLayout().getAllocaAddrSpace());
+ auto DestAS = getContext().getTargetAddressSpace(DestLangAS);
+ auto *T = DeclPtr.getElementType()->getPointerTo(DestAS);
+ DeclPtr = DeclPtr.withPointer(getTargetHooks().performAddrSpaceCast(
+ *this, V, SrcLangAS, DestLangAS, T, true));
+ }
+
+ // Push a destructor cleanup for this parameter if the ABI requires it.
+ // Don't push a cleanup in a thunk for a method that will also emit a
+ // cleanup.
+ if (Ty->isRecordType() && !CurFuncIsThunk &&
+ Ty->castAs<RecordType>()->getDecl()->isParamDestroyedInCallee()) {
+ if (QualType::DestructionKind DtorKind =
+ D.needsDestruction(getContext())) {
+ assert((DtorKind == QualType::DK_cxx_destructor ||
+ DtorKind == QualType::DK_nontrivial_c_struct) &&
+ "unexpected destructor type");
+ pushDestroy(DtorKind, DeclPtr, Ty);
+ CalleeDestructedParamCleanups[cast<ParmVarDecl>(&D)] =
+ EHStack.stable_begin();
+ }
+ }
+ } else {
+ // Create a temporary to hold the value.
+ DeclPtr = CreateMemTemp(Ty, getContext().getDeclAlign(&D),
+ D.getName() + ".addr", &AllocaPtr);
+ DoCopy = true;
+ }
+ }
- LValue lv = MakeAddrLValue(DeclPtr, Ty);
if (IsScalar) {
Qualifiers qs = Ty.getQualifiers();
if (Qualifiers::ObjCLifetime lt = qs.getObjCLifetime()) {
@@ -2547,6 +2548,9 @@
lt = Qualifiers::OCL_ExplicitNone;
}
+ llvm::Value *ArgVal = (DoStore ? Arg.getDirectValue() : nullptr);
+ LValue lv = MakeAddrLValue(DeclPtr, Ty);
+
// Load objects passed indirectly.
if (Arg.isIndirect() && !ArgVal)
ArgVal = Builder.CreateLoad(DeclPtr);
@@ -2588,9 +2592,18 @@
}
}
- // Store the initial value into the alloca.
- if (DoStore)
- EmitStoreOfScalar(ArgVal, lv, /* isInitialization */ true);
+ // There are two cases when a copy is needed:
+ // 1. The parameter is controlled by OpenMP (globalized).
+ // 2. The parameter is direct.
+ if (DoCopy) {
+ LValue Dst = MakeAddrLValue(DeclPtr, Ty);
+ if (Arg.isIndirect()) {
+ LValue Src = MakeAddrLValue(Arg.getIndirectAddress(), Ty);
+ callCStructCopyConstructor(Dst, Src);
+ } else {
+ EmitStoreOfScalar(Arg.getDirectValue(), Dst, /* isInitialization */ true);
+ }
+ }
setAddrOfLocalVar(&D, DeclPtr);
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
