Revert "[OPENMP]Fix PR37671: Privatize local(private) variables in untied tasks."

This reverts commit ec9563c54e to
investigate compiler crash revelaed by the buildbots.
This commit is contained in:
Alexey Bataev 2020-08-12 09:49:59 -04:00
parent e891b6a75d
commit 3651658bdd
4 changed files with 77 additions and 225 deletions

View File

@ -180,7 +180,7 @@ public:
UntiedCodeGen(CGF);
CodeGenFunction::JumpDest CurPoint =
CGF.getJumpDestInCurrentScope(".untied.next.");
CGF.EmitBranch(CGF.ReturnBlock.getBlock());
CGF.EmitBranchThroughCleanup(CGF.ReturnBlock);
CGF.EmitBlock(CGF.createBasicBlock(".untied.jmp."));
UntiedSwitch->addCase(CGF.Builder.getInt32(UntiedSwitch->getNumCases()),
CGF.Builder.GetInsertBlock());
@ -3370,7 +3370,6 @@ struct PrivateHelpersTy {
const VarDecl *PrivateCopy, const VarDecl *PrivateElemInit)
: OriginalRef(OriginalRef), Original(Original), PrivateCopy(PrivateCopy),
PrivateElemInit(PrivateElemInit) {}
PrivateHelpersTy(const VarDecl *Original) : Original(Original) {}
const Expr *OriginalRef = nullptr;
const VarDecl *Original = nullptr;
const VarDecl *PrivateCopy = nullptr;
@ -3391,10 +3390,6 @@ createPrivatesRecordDecl(CodeGenModule &CGM, ArrayRef<PrivateDataTy> Privates) {
for (const auto &Pair : Privates) {
const VarDecl *VD = Pair.second.Original;
QualType Type = VD->getType().getNonReferenceType();
// If the private variable is a local variable with lvalue ref type,
// allocate the pointer instead of the pointee type.
if (!Pair.second.OriginalRef && VD->getType()->isLValueReferenceType())
Type = C.getPointerType(Type);
FieldDecl *FD = addFieldToRecordDecl(C, RD, Type);
if (VD->hasAttrs()) {
for (specific_attr_iterator<AlignedAttr> I(VD->getAttrs().begin()),
@ -3648,7 +3643,10 @@ static llvm::Value *emitDestructorsFunction(CodeGenModule &CGM,
/// \endcode
static llvm::Value *
emitTaskPrivateMappingFunction(CodeGenModule &CGM, SourceLocation Loc,
const OMPTaskDataTy &Data, QualType PrivatesQTy,
ArrayRef<const Expr *> PrivateVars,
ArrayRef<const Expr *> FirstprivateVars,
ArrayRef<const Expr *> LastprivateVars,
QualType PrivatesQTy,
ArrayRef<PrivateDataTy> Privates) {
ASTContext &C = CGM.getContext();
FunctionArgList Args;
@ -3657,9 +3655,9 @@ emitTaskPrivateMappingFunction(CodeGenModule &CGM, SourceLocation Loc,
C.getPointerType(PrivatesQTy).withConst().withRestrict(),
ImplicitParamDecl::Other);
Args.push_back(&TaskPrivatesArg);
llvm::DenseMap<CanonicalDeclPtr<const VarDecl>, unsigned> PrivateVarsPos;
llvm::DenseMap<const VarDecl *, unsigned> PrivateVarsPos;
unsigned Counter = 1;
for (const Expr *E : Data.PrivateVars) {
for (const Expr *E : PrivateVars) {
Args.push_back(ImplicitParamDecl::Create(
C, /*DC=*/nullptr, Loc, /*Id=*/nullptr,
C.getPointerType(C.getPointerType(E->getType()))
@ -3670,7 +3668,7 @@ emitTaskPrivateMappingFunction(CodeGenModule &CGM, SourceLocation Loc,
PrivateVarsPos[VD] = Counter;
++Counter;
}
for (const Expr *E : Data.FirstprivateVars) {
for (const Expr *E : FirstprivateVars) {
Args.push_back(ImplicitParamDecl::Create(
C, /*DC=*/nullptr, Loc, /*Id=*/nullptr,
C.getPointerType(C.getPointerType(E->getType()))
@ -3681,7 +3679,7 @@ emitTaskPrivateMappingFunction(CodeGenModule &CGM, SourceLocation Loc,
PrivateVarsPos[VD] = Counter;
++Counter;
}
for (const Expr *E : Data.LastprivateVars) {
for (const Expr *E : LastprivateVars) {
Args.push_back(ImplicitParamDecl::Create(
C, /*DC=*/nullptr, Loc, /*Id=*/nullptr,
C.getPointerType(C.getPointerType(E->getType()))
@ -3692,17 +3690,6 @@ emitTaskPrivateMappingFunction(CodeGenModule &CGM, SourceLocation Loc,
PrivateVarsPos[VD] = Counter;
++Counter;
}
for (const VarDecl *VD : Data.PrivateLocals) {
QualType Ty = VD->getType().getNonReferenceType();
if (VD->getType()->isLValueReferenceType())
Ty = C.getPointerType(Ty);
Args.push_back(ImplicitParamDecl::Create(
C, /*DC=*/nullptr, Loc, /*Id=*/nullptr,
C.getPointerType(C.getPointerType(Ty)).withConst().withRestrict(),
ImplicitParamDecl::Other));
PrivateVarsPos[VD] = Counter;
++Counter;
}
const auto &TaskPrivatesMapFnInfo =
CGM.getTypes().arrangeBuiltinFunctionDeclaration(C.VoidTy, Args);
llvm::FunctionType *TaskPrivatesMapTy =
@ -3958,16 +3945,16 @@ emitTaskDupFunction(CodeGenModule &CGM, SourceLocation Loc,
/// Checks if destructor function is required to be generated.
/// \return true if cleanups are required, false otherwise.
static bool
checkDestructorsRequired(const RecordDecl *KmpTaskTWithPrivatesQTyRD,
ArrayRef<PrivateDataTy> Privates) {
for (const PrivateDataTy &P : Privates) {
if (!P.second.OriginalRef)
continue;
QualType Ty = P.second.Original->getType().getNonReferenceType();
if (Ty.isDestructedType())
return true;
checkDestructorsRequired(const RecordDecl *KmpTaskTWithPrivatesQTyRD) {
bool NeedsCleanup = false;
auto FI = std::next(KmpTaskTWithPrivatesQTyRD->field_begin(), 1);
const auto *PrivateRD = cast<RecordDecl>(FI->getType()->getAsTagDecl());
for (const FieldDecl *FD : PrivateRD->fields()) {
NeedsCleanup = NeedsCleanup || FD->getType().isDestructedType();
if (NeedsCleanup)
break;
}
return false;
return NeedsCleanup;
}
namespace {
@ -4137,10 +4124,7 @@ CGOpenMPRuntime::emitTaskInit(CodeGenFunction &CGF, SourceLocation Loc,
/*PrivateElemInit=*/nullptr));
++I;
}
for (const VarDecl *VD : Data.PrivateLocals)
Privates.emplace_back(C.getDeclAlign(VD), PrivateHelpersTy(VD));
llvm::stable_sort(Privates,
[](const PrivateDataTy &L, const PrivateDataTy &R) {
llvm::stable_sort(Privates, [](PrivateDataTy L, PrivateDataTy R) {
return L.first > R.first;
});
QualType KmpInt32Ty = C.getIntTypeForBitwidth(/*DestWidth=*/32, /*Signed=*/1);
@ -4184,8 +4168,9 @@ CGOpenMPRuntime::emitTaskInit(CodeGenFunction &CGF, SourceLocation Loc,
std::next(TaskFunction->arg_begin(), 3)->getType();
if (!Privates.empty()) {
auto FI = std::next(KmpTaskTWithPrivatesQTyRD->field_begin());
TaskPrivatesMap =
emitTaskPrivateMappingFunction(CGM, Loc, Data, FI->getType(), Privates);
TaskPrivatesMap = emitTaskPrivateMappingFunction(
CGM, Loc, Data.PrivateVars, Data.FirstprivateVars, Data.LastprivateVars,
FI->getType(), Privates);
TaskPrivatesMap = CGF.Builder.CreatePointerBitCastOrAddrSpaceCast(
TaskPrivatesMap, TaskPrivatesMapTy);
} else {
@ -4215,8 +4200,7 @@ CGOpenMPRuntime::emitTaskInit(CodeGenFunction &CGF, SourceLocation Loc,
unsigned Flags = Data.Tied ? TiedFlag : 0;
bool NeedsCleanup = false;
if (!Privates.empty()) {
NeedsCleanup =
checkDestructorsRequired(KmpTaskTWithPrivatesQTyRD, Privates);
NeedsCleanup = checkDestructorsRequired(KmpTaskTWithPrivatesQTyRD);
if (NeedsCleanup)
Flags = Flags | DestructorsFlag;
}
@ -11239,7 +11223,8 @@ Address CGOpenMPRuntime::getAddressOfLocalVariable(CodeGenFunction &CGF,
if (!VD)
return Address::invalid();
const VarDecl *CVD = VD->getCanonicalDecl();
if (CVD->hasAttr<OMPAllocateDeclAttr>()) {
if (!CVD->hasAttr<OMPAllocateDeclAttr>())
return Address::invalid();
const auto *AA = CVD->getAttr<OMPAllocateDeclAttr>();
// Use the default allocation.
if ((AA->getAllocatorType() == OMPAllocateDeclAttr::OMPDefaultMemAlloc ||
@ -11263,13 +11248,13 @@ Address CGOpenMPRuntime::getAddressOfLocalVariable(CodeGenFunction &CGF,
assert(AA->getAllocator() &&
"Expected allocator expression for non-default allocator.");
llvm::Value *Allocator = CGF.EmitScalarExpr(AA->getAllocator());
// According to the standard, the original allocator type is a enum
// (integer). Convert to pointer type, if required.
// According to the standard, the original allocator type is a enum (integer).
// Convert to pointer type, if required.
if (Allocator->getType()->isIntegerTy())
Allocator = CGF.Builder.CreateIntToPtr(Allocator, CGM.VoidPtrTy);
else if (Allocator->getType()->isPointerTy())
Allocator = CGF.Builder.CreatePointerBitCastOrAddrSpaceCast(
Allocator, CGM.VoidPtrTy);
Allocator = CGF.Builder.CreatePointerBitCastOrAddrSpaceCast(Allocator,
CGM.VoidPtrTy);
llvm::Value *Args[] = {ThreadID, Size, Allocator};
llvm::Value *Addr =
@ -11289,15 +11274,6 @@ Address CGOpenMPRuntime::getAddressOfLocalVariable(CodeGenFunction &CGF,
getName({CVD->getName(), ".addr"}));
return Address(Addr, Align);
}
if (UntiedLocalVarsStack.empty())
return Address::invalid();
const UntiedLocalVarsAddressesMap &UntiedData = UntiedLocalVarsStack.back();
auto It = UntiedData.find(VD);
if (It == UntiedData.end())
return Address::invalid();
return It->second;
}
CGOpenMPRuntime::NontemporalDeclsRAII::NontemporalDeclsRAII(
CodeGenModule &CGM, const OMPLoopDirective &S)
@ -11331,21 +11307,6 @@ CGOpenMPRuntime::NontemporalDeclsRAII::~NontemporalDeclsRAII() {
CGM.getOpenMPRuntime().NontemporalDeclsStack.pop_back();
}
CGOpenMPRuntime::UntiedTaskLocalDeclsRAII::UntiedTaskLocalDeclsRAII(
CodeGenModule &CGM,
const llvm::DenseMap<CanonicalDeclPtr<const VarDecl>, Address> &LocalVars)
: CGM(CGM), NeedToPush(!LocalVars.empty()) {
if (!NeedToPush)
return;
CGM.getOpenMPRuntime().UntiedLocalVarsStack.push_back(LocalVars);
}
CGOpenMPRuntime::UntiedTaskLocalDeclsRAII::~UntiedTaskLocalDeclsRAII() {
if (!NeedToPush)
return;
CGM.getOpenMPRuntime().UntiedLocalVarsStack.pop_back();
}
bool CGOpenMPRuntime::isNontemporalDecl(const ValueDecl *VD) const {
assert(CGM.getLangOpts().OpenMP && "Not in OpenMP mode.");

View File

@ -105,7 +105,6 @@ struct OMPTaskDataTy final {
SmallVector<const Expr *, 4> ReductionOrigs;
SmallVector<const Expr *, 4> ReductionCopies;
SmallVector<const Expr *, 4> ReductionOps;
SmallVector<CanonicalDeclPtr<const VarDecl>, 4> PrivateLocals;
struct DependData {
OpenMPDependClauseKind DepKind = OMPC_DEPEND_unknown;
const Expr *IteratorExpr = nullptr;
@ -246,19 +245,6 @@ public:
~NontemporalDeclsRAII();
};
/// Manages list of nontemporal decls for the specified directive.
class UntiedTaskLocalDeclsRAII {
CodeGenModule &CGM;
const bool NeedToPush;
public:
UntiedTaskLocalDeclsRAII(
CodeGenModule &CGM,
const llvm::DenseMap<CanonicalDeclPtr<const VarDecl>, Address>
&LocalVars);
~UntiedTaskLocalDeclsRAII();
};
/// Maps the expression for the lastprivate variable to the global copy used
/// to store new value because original variables are not mapped in inner
/// parallel regions. Only private copies are captured but we need also to
@ -719,10 +705,6 @@ private:
/// The set is the union of all current stack elements.
llvm::SmallVector<NontemporalDeclsSet, 4> NontemporalDeclsStack;
using UntiedLocalVarsAddressesMap =
llvm::DenseMap<CanonicalDeclPtr<const VarDecl>, Address>;
llvm::SmallVector<UntiedLocalVarsAddressesMap, 4> UntiedLocalVarsStack;
/// Stack for list of addresses of declarations in current context marked as
/// lastprivate conditional. The set is the union of all current stack
/// elements.

View File

@ -21,7 +21,6 @@
#include "clang/AST/OpenMPClause.h"
#include "clang/AST/Stmt.h"
#include "clang/AST/StmtOpenMP.h"
#include "clang/AST/StmtVisitor.h"
#include "clang/Basic/OpenMPKinds.h"
#include "clang/Basic/PrettyStackTrace.h"
#include "llvm/Frontend/OpenMP/OMPConstants.h"
@ -3785,42 +3784,6 @@ void CodeGenFunction::EmitOMPParallelSectionsDirective(
checkForLastprivateConditionalUpdate(*this, S);
}
namespace {
/// Get the list of variables declared in the context of the untied tasks.
class CheckVarsEscapingUntiedTaskDeclContext final
: public ConstStmtVisitor<CheckVarsEscapingUntiedTaskDeclContext> {
llvm::SmallVector<const VarDecl *, 4> PrivateDecls;
public:
explicit CheckVarsEscapingUntiedTaskDeclContext() = default;
virtual ~CheckVarsEscapingUntiedTaskDeclContext() = default;
void VisitDeclStmt(const DeclStmt *S) {
if (!S)
return;
// Need to privatize only local vars, static locals can be processed as is.
for (const Decl *D : S->decls()) {
if (const auto *VD = dyn_cast_or_null<VarDecl>(D))
if (VD->hasLocalStorage())
PrivateDecls.push_back(VD);
}
}
void VisitOMPExecutableDirective(const OMPExecutableDirective *) { return; }
void VisitCapturedStmt(const CapturedStmt *) { return; }
void VisitLambdaExpr(const LambdaExpr *) { return; }
void VisitBlockExpr(const BlockExpr *) { return; }
void VisitStmt(const Stmt *S) {
if (!S)
return;
for (const Stmt *Child : S->children())
if (Child)
Visit(Child);
}
/// Swaps list of vars with the provided one.
ArrayRef<const VarDecl *> getPrivateDecls() const { return PrivateDecls; }
};
} // anonymous namespace
void CodeGenFunction::EmitOMPTaskBasedDirective(
const OMPExecutableDirective &S, const OpenMPDirectiveKind CapturedRegion,
const RegionCodeGenTy &BodyGen, const TaskGenTy &TaskGen,
@ -3921,22 +3884,14 @@ void CodeGenFunction::EmitOMPTaskBasedDirective(
Data.Dependences.emplace_back(C->getDependencyKind(), C->getModifier());
DD.DepExprs.append(C->varlist_begin(), C->varlist_end());
}
// Get list of local vars for untied tasks.
if (!Data.Tied) {
CheckVarsEscapingUntiedTaskDeclContext Checker;
Checker.Visit(S.getInnermostCapturedStmt()->getCapturedStmt());
Data.PrivateLocals.append(Checker.getPrivateDecls().begin(),
Checker.getPrivateDecls().end());
}
auto &&CodeGen = [&Data, &S, CS, &BodyGen, &LastprivateDstsOrigs,
CapturedRegion](CodeGenFunction &CGF,
PrePostActionTy &Action) {
llvm::DenseMap<CanonicalDeclPtr<const VarDecl>, Address> UntiedLocalVars;
// Set proper addresses for generated private copies.
OMPPrivateScope Scope(CGF);
llvm::SmallVector<std::pair<const VarDecl *, Address>, 16> FirstprivatePtrs;
if (!Data.PrivateVars.empty() || !Data.FirstprivateVars.empty() ||
!Data.LastprivateVars.empty() || !Data.PrivateLocals.empty()) {
!Data.LastprivateVars.empty()) {
llvm::FunctionType *CopyFnTy = llvm::FunctionType::get(
CGF.Builder.getVoidTy(), {CGF.Builder.getInt8PtrTy()}, true);
enum { PrivatesParam = 2, CopyFnParam = 3 };
@ -3972,15 +3927,6 @@ void CodeGenFunction::EmitOMPTaskBasedDirective(
PrivatePtrs.emplace_back(VD, PrivatePtr);
CallArgs.push_back(PrivatePtr.getPointer());
}
for (const VarDecl *VD : Data.PrivateLocals) {
QualType Ty = VD->getType().getNonReferenceType();
if (VD->getType()->isLValueReferenceType())
Ty = CGF.getContext().getPointerType(Ty);
Address PrivatePtr = CGF.CreateMemTemp(
CGF.getContext().getPointerType(Ty), ".local.ptr.addr");
UntiedLocalVars.try_emplace(VD, PrivatePtr);
CallArgs.push_back(PrivatePtr.getPointer());
}
CGF.CGM.getOpenMPRuntime().emitOutlinedFunctionCall(
CGF, S.getBeginLoc(), {CopyFnTy, CopyFn}, CallArgs);
for (const auto &Pair : LastprivateDstsOrigs) {
@ -3999,13 +3945,6 @@ void CodeGenFunction::EmitOMPTaskBasedDirective(
CGF.getContext().getDeclAlign(Pair.first));
Scope.addPrivate(Pair.first, [Replacement]() { return Replacement; });
}
// Adjust mapping for internal locals by mapping actual memory instead of
// a pointer to this memory.
for (auto &Pair : UntiedLocalVars) {
Address Replacement(CGF.Builder.CreateLoad(Pair.second),
CGF.getContext().getDeclAlign(Pair.first));
Pair.getSecond() = Replacement;
}
}
if (Data.Reductions) {
OMPPrivateScope FirstprivateScope(CGF);
@ -4100,8 +4039,6 @@ void CodeGenFunction::EmitOMPTaskBasedDirective(
}
(void)InRedScope.Privatize();
CGOpenMPRuntime::UntiedTaskLocalDeclsRAII LocalVarsScope(CGF.CGM,
UntiedLocalVars);
Action.Enter(CGF);
BodyGen(CGF);
};

View File

@ -1,4 +1,4 @@
// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -x c++ -emit-llvm %s -o - | FileCheck %s --check-prefix CHECK --check-prefix UNTIEDRT
// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -x c++ -emit-llvm %s -o - | FileCheck %s
// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s
// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
//
@ -258,7 +258,7 @@ int main() {
a = 4;
c = 5;
}
// CHECK: [[ORIG_TASK_PTR:%.+]] = call i8* @__kmpc_omp_task_alloc([[IDENT_T]]* @{{.+}}, i32 {{%.*}}, i32 0, i64 48, i64 1, i32 (i32, i8*)* bitcast (i32 (i32, [[KMP_TASK_T]]{{.*}}*)* [[TASK_ENTRY6:@.+]] to i32 (i32, i8*)*))
// CHECK: [[ORIG_TASK_PTR:%.+]] = call i8* @__kmpc_omp_task_alloc([[IDENT_T]]* @{{.+}}, i32 {{%.*}}, i32 0, i64 40, i64 1, i32 (i32, i8*)* bitcast (i32 (i32, [[KMP_TASK_T]]{{.*}}*)* [[TASK_ENTRY6:@.+]] to i32 (i32, i8*)*))
// CHECK: call i32 @__kmpc_omp_task([[IDENT_T]]* @{{.+}}, i32 {{%.*}}, i8* [[ORIG_TASK_PTR]])
#pragma omp task untied
{
@ -295,54 +295,26 @@ int main() {
// CHECK: store i32 4, i32* [[A_PTR]]
// CHECK: define internal i32 [[TASK_ENTRY6]](i32 %0, [[KMP_TASK_T]]{{.*}}* noalias %1)
// UNTIEDRT: [[S1_ADDR_PTR:%.+]] = alloca %struct.S*,
// UNTIEDRT: call void (i8*, ...) %{{.+}}(i8* %{{.+}}, %struct.S** [[S1_ADDR_PTR]])
// UNTIEDRT: [[S1_ADDR:%.+]] = load %struct.S*, %struct.S** [[S1_ADDR_PTR]],
// CHECK: switch i32 %{{.+}}, label %[[DONE:.+]] [
// CHECK: [[DONE]]:
// CHECK: br label %[[CLEANUP:[^,]+]]
// CHECK: switch i32 %{{.+}}, label
// CHECK: load i32*, i32** %
// CHECK: store i32 1, i32* %
// CHECK: call i32 @__kmpc_omp_task(%
// UNTIEDRT: br label %[[EXIT:[^,]+]]
// UNTIEDRT: call void [[CONSTR:@.+]](%struct.S* [[S1_ADDR]])
// CHECK: call i8* @__kmpc_omp_task_alloc(
// CHECK: call i32 @__kmpc_omp_task(%
// CHECK: load i32*, i32** %
// CHECK: store i32 2, i32* %
// CHECK: call i32 @__kmpc_omp_task(%
// UNTIEDRT: br label %[[EXIT]]
// CHECK: call i32 @__kmpc_omp_taskyield(%
// CHECK: load i32*, i32** %
// CHECK: store i32 3, i32* %
// CHECK: call i32 @__kmpc_omp_task(%
// UNTIEDRT: br label %[[EXIT]]
// s1 = S();
// UNTIEDRT: call void [[CONSTR]](%struct.S* [[TMP:%.+]])
// UNTIEDRT: [[DST:%.+]] = bitcast %struct.S* [[S1_ADDR]] to i8*
// UNTIEDRT: [[SRC:%.+]] = bitcast %struct.S* [[TMP]] to i8*
// UNTIEDRT: call void @llvm.memcpy.{{.+}}(i8* {{.*}}[[DST]], i8* {{.*}}[[SRC]], i64 4, i1 false)
// UNTIEDRT: call void [[DESTR:@.+]](%struct.S* [[TMP]])
// CHECK: call i32 @__kmpc_omp_taskwait(%
// CHECK: load i32*, i32** %
// CHECK: store i32 4, i32* %
// CHECK: call i32 @__kmpc_omp_task(%
// UNTIEDRT: br label %[[EXIT]]
// UNTIEDRT: call void [[DESTR]](%struct.S* [[S1_ADDR]])
// CHECK: br label %[[CLEANUP]]
// CHECK: [[CLEANUP]]:
// UNTIEDRT: br label %[[EXIT]]
// UNTIEDRT: [[EXIT]]:
// UNTIEDRT-NEXT: ret i32 0
struct S1 {
int a;