Reapply "[OpenACC] Sema changes for +*&|^ reduction combiner recipes … #163205
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…(… (llvm#162920) This reverts commit 8d9aecc. That was reverted because of an ASAN failure that I believe this fixes. The std::uninitialized_copy of an array of a type that contained a llvm::vector using operator= instead of the copy constructor, so it made ASan think it was overwriting existing data? However, it also probably would have tried to do some sort of deallocation if had the 'right' amount of wrong data. So this version switches over to using placement-new instead.
llvmbot
added
clang
|
@llvm/pr-subscribers-clangir @llvm/pr-subscribers-clang Author: Erich Keane (erichkeane) Changes…(… (#162920) This reverts commit 8d9aecc. That was reverted because of an ASAN failure that I believe this fixes. The std::uninitialized_copy of an array of a type that contained a llvm::vector using operator= instead of the copy constructor, so it made ASan think it was overwriting existing data? However, it also probably would have tried to do some sort of deallocation if had the 'right' amount of wrong data. So this version switches over to using placement-new instead. Patch is 298.08 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/163205.diff 17 Files Affected:
diff --git a/clang/include/clang/AST/OpenACCClause.h b/clang/include/clang/AST/OpenACCClause.h
index 79cffeb4dade9..ab2bf0e08692c 100644
--- a/clang/include/clang/AST/OpenACCClause.h
+++ b/clang/include/clang/AST/OpenACCClause.h
@@ -1325,8 +1325,14 @@ class OpenACCReductionClause final
Op(Operator) {
assert(VarList.size() == Recipes.size());
setExprs(getTrailingObjects<Expr *>(VarList.size()), VarList);
- llvm::uninitialized_copy(Recipes, getTrailingObjects<
- OpenACCReductionRecipe > ());
+
+ // Initialize the recipe list.
+ unsigned I = 0;
+ for (const OpenACCReductionRecipe &R : Recipes) {
+ new (&getTrailingObjects<OpenACCReductionRecipe>()[I])
+ OpenACCReductionRecipe(R);
+ ++I;
+ }
}
public:
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3df28f2ef3334..40bc7b9a4e45e 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -13665,6 +13665,11 @@ def warn_acc_var_referenced_lacks_op
"reference has no effect">,
InGroup<DiagGroup<"openacc-var-lacks-operation">>,
DefaultError;
+def err_acc_reduction_recipe_no_op
+ : Error<"variable of type %0 referenced in OpenACC 'reduction' clause does "
+ "not have a valid operation available">;
+def note_acc_reduction_recipe_noop_field
+ : Note<"while forming combiner for compound type %0">;
// AMDGCN builtins diagnostics
def err_amdgcn_load_lds_size_invalid_value : Error<"invalid size value">;
diff --git a/clang/include/clang/Sema/SemaOpenACC.h b/clang/include/clang/Sema/SemaOpenACC.h
index 6cadc343cd728..16e7f1bc83019 100644
--- a/clang/include/clang/Sema/SemaOpenACC.h
+++ b/clang/include/clang/Sema/SemaOpenACC.h
@@ -228,6 +228,11 @@ class SemaOpenACC : public SemaBase {
bool DiagnoseAllowedClauses(OpenACCDirectiveKind DK, OpenACCClauseKind CK,
SourceLocation ClauseLoc);
+ bool CreateReductionCombinerRecipe(
+ SourceLocation loc, OpenACCReductionOperator ReductionOperator,
+ QualType VarTy,
+ llvm::SmallVectorImpl<OpenACCReductionRecipe::CombinerRecipe>
+ &CombinerRecipes);
public:
// Needed from the visitor, so should be public.
diff --git a/clang/lib/Sema/SemaOpenACC.cpp b/clang/lib/Sema/SemaOpenACC.cpp
index f3969a96b8ced..779b6e9cb6b4d 100644
--- a/clang/lib/Sema/SemaOpenACC.cpp
+++ b/clang/lib/Sema/SemaOpenACC.cpp
@@ -2898,6 +2898,15 @@ OpenACCReductionRecipe SemaOpenACC::CreateReductionInitRecipe(
dyn_cast<ArraySectionExpr>(VarExpr->IgnoreParenImpCasts()))
VarTy = ASE->getElementType();
+ llvm::SmallVector<OpenACCReductionRecipe::CombinerRecipe, 1> CombinerRecipes;
+
+ // We use the 'set-ness' of the alloca-decl to determine whether the combiner
+ // is 'set' or not, so we can skip any attempts at it if we're going to fail
+ // at any of the combiners.
+ if (CreateReductionCombinerRecipe(VarExpr->getBeginLoc(), ReductionOperator,
+ VarTy, CombinerRecipes))
+ return OpenACCReductionRecipe::Empty();
+
VarDecl *AllocaDecl = CreateAllocaDecl(
getASTContext(), SemaRef.getCurContext(), VarExpr->getBeginLoc(),
&getASTContext().Idents.get("openacc.reduction.init"), VarTy);
@@ -2946,5 +2955,163 @@ OpenACCReductionRecipe SemaOpenACC::CreateReductionInitRecipe(
AllocaDecl->setInit(Init.get());
AllocaDecl->setInitStyle(VarDecl::CallInit);
}
- return OpenACCReductionRecipe(AllocaDecl, {});
+
+ return OpenACCReductionRecipe(AllocaDecl, CombinerRecipes);
+}
+
+bool SemaOpenACC::CreateReductionCombinerRecipe(
+ SourceLocation Loc, OpenACCReductionOperator ReductionOperator,
+ QualType VarTy,
+ llvm::SmallVectorImpl<OpenACCReductionRecipe::CombinerRecipe>
+ &CombinerRecipes) {
+ // Now we can try to generate the 'combiner' recipe. This is a little
+ // complicated in that if the 'VarTy' is an array type, we want to take its
+ // element type so we can generate that. Additionally, if this is a struct,
+ // we have two options: If there is overloaded operators, we want to take
+ // THOSE, else we want to do the individual elements.
+
+ BinaryOperatorKind BinOp;
+ switch (ReductionOperator) {
+ case OpenACCReductionOperator::Invalid:
+ // This can only happen when there is an error, and since these inits
+ // are used for code generation, we can just ignore/not bother doing any
+ // initialization here.
+ CombinerRecipes.push_back({nullptr, nullptr, nullptr});
+ return false;
+ case OpenACCReductionOperator::Addition:
+ BinOp = BinaryOperatorKind::BO_AddAssign;
+ break;
+ case OpenACCReductionOperator::Multiplication:
+ BinOp = BinaryOperatorKind::BO_MulAssign;
+ break;
+ case OpenACCReductionOperator::BitwiseAnd:
+ BinOp = BinaryOperatorKind::BO_AndAssign;
+ break;
+ case OpenACCReductionOperator::BitwiseOr:
+ BinOp = BinaryOperatorKind::BO_OrAssign;
+ break;
+ case OpenACCReductionOperator::BitwiseXOr:
+ BinOp = BinaryOperatorKind::BO_XorAssign;
+ break;
+
+ case OpenACCReductionOperator::Max:
+ case OpenACCReductionOperator::Min:
+ case OpenACCReductionOperator::And:
+ case OpenACCReductionOperator::Or:
+ // We just want a 'NYI' error in the backend, so leave an empty combiner
+ // recipe, and claim success.
+ CombinerRecipes.push_back({nullptr, nullptr, nullptr});
+ return false;
+ }
+
+ // If VarTy is an array type, at the top level only, we want to do our
+ // compares/decomp/etc at the element level.
+ if (auto *AT = getASTContext().getAsArrayType(VarTy))
+ VarTy = AT->getElementType();
+
+ assert(!VarTy->isArrayType() && "Only 1 level of array allowed");
+
+ auto tryCombiner = [&, this](DeclRefExpr *LHSDRE, DeclRefExpr *RHSDRE,
+ bool IncludeTrap) {
+ // TODO: OpenACC: we have to figure out based on the bin-op how to do the
+ // ones that we can't just use compound operators for. So &&, ||, max, and
+ // min aren't really clear what we could do here.
+ if (IncludeTrap) {
+ // Trap all of the errors here, we'll emit our own at the end.
+ Sema::TentativeAnalysisScope Trap{SemaRef};
+
+ return SemaRef.BuildBinOp(SemaRef.getCurScope(), Loc, BinOp, LHSDRE,
+ RHSDRE,
+ /*ForFoldExpr=*/false);
+ } else {
+ return SemaRef.BuildBinOp(SemaRef.getCurScope(), Loc, BinOp, LHSDRE,
+ RHSDRE,
+ /*ForFoldExpr=*/false);
+ }
+ };
+
+ struct CombinerAttemptTy {
+ VarDecl *LHS;
+ DeclRefExpr *LHSDRE;
+ VarDecl *RHS;
+ DeclRefExpr *RHSDRE;
+ Expr *Op;
+ };
+
+ auto formCombiner = [&, this](QualType Ty) -> CombinerAttemptTy {
+ VarDecl *LHSDecl = CreateAllocaDecl(
+ getASTContext(), SemaRef.getCurContext(), Loc,
+ &getASTContext().Idents.get("openacc.reduction.combiner.lhs"), Ty);
+ auto *LHSDRE = DeclRefExpr::Create(
+ getASTContext(), NestedNameSpecifierLoc{}, SourceLocation{}, LHSDecl,
+ /*ReferstoEnclosingVariableOrCapture=*/false,
+ DeclarationNameInfo{DeclarationName{LHSDecl->getDeclName()},
+ LHSDecl->getBeginLoc()},
+ Ty, clang::VK_LValue, LHSDecl, nullptr, NOUR_None);
+ VarDecl *RHSDecl = CreateAllocaDecl(
+ getASTContext(), SemaRef.getCurContext(), Loc,
+ &getASTContext().Idents.get("openacc.reduction.combiner.lhs"), Ty);
+ auto *RHSDRE = DeclRefExpr::Create(
+ getASTContext(), NestedNameSpecifierLoc{}, SourceLocation{}, RHSDecl,
+ /*ReferstoEnclosingVariableOrCapture=*/false,
+ DeclarationNameInfo{DeclarationName{RHSDecl->getDeclName()},
+ RHSDecl->getBeginLoc()},
+ Ty, clang::VK_LValue, RHSDecl, nullptr, NOUR_None);
+
+ ExprResult BinOpResult = tryCombiner(LHSDRE, RHSDRE, /*IncludeTrap=*/true);
+
+ return {LHSDecl, LHSDRE, RHSDecl, RHSDRE, BinOpResult.get()};
+ };
+
+ CombinerAttemptTy TopLevelCombinerInfo = formCombiner(VarTy);
+
+ if (TopLevelCombinerInfo.Op) {
+ if (!TopLevelCombinerInfo.Op->containsErrors() &&
+ TopLevelCombinerInfo.Op->isInstantiationDependent()) {
+ // If this is instantiation dependent, we're just going to 'give up' here
+ // and count on us to get it right during instantaition.
+ CombinerRecipes.push_back({nullptr, nullptr, nullptr});
+ return false;
+ } else if (!TopLevelCombinerInfo.Op->containsErrors()) {
+ // Else, we succeeded, we can just return this combiner.
+ CombinerRecipes.push_back({TopLevelCombinerInfo.LHS,
+ TopLevelCombinerInfo.RHS,
+ TopLevelCombinerInfo.Op});
+ return false;
+ }
+ }
+
+ // Since the 'root' level didn't fail, the only thing that could be successful
+ // is a struct that we decompose on its individual fields.
+
+ RecordDecl *RD = VarTy->getAsRecordDecl();
+ if (!RD) {
+ Diag(Loc, diag::err_acc_reduction_recipe_no_op) << VarTy;
+ tryCombiner(TopLevelCombinerInfo.LHSDRE, TopLevelCombinerInfo.RHSDRE,
+ /*IncludeTrap=*/false);
+ return true;
+ }
+
+ for (const FieldDecl *FD : RD->fields()) {
+ CombinerAttemptTy FieldCombinerInfo = formCombiner(FD->getType());
+
+ if (!FieldCombinerInfo.Op || FieldCombinerInfo.Op->containsErrors()) {
+ Diag(Loc, diag::err_acc_reduction_recipe_no_op) << FD->getType();
+ Diag(FD->getBeginLoc(), diag::note_acc_reduction_recipe_noop_field) << RD;
+ tryCombiner(FieldCombinerInfo.LHSDRE, FieldCombinerInfo.RHSDRE,
+ /*IncludeTrap=*/false);
+ return true;
+ }
+
+ if (FieldCombinerInfo.Op->isInstantiationDependent()) {
+ // If this is instantiation dependent, we're just going to 'give up' here
+ // and count on us to get it right during instantaition.
+ CombinerRecipes.push_back({nullptr, nullptr, nullptr});
+ } else {
+ CombinerRecipes.push_back(
+ {FieldCombinerInfo.LHS, FieldCombinerInfo.RHS, FieldCombinerInfo.Op});
+ }
+ }
+
+ return false;
}
diff --git a/clang/test/CIR/CodeGenOpenACC/combined-reduction-clause-default-ops.cpp b/clang/test/CIR/CodeGenOpenACC/combined-reduction-clause-default-ops.cpp
index 7b74b7cee1e75..040ddd3ca458c 100644
--- a/clang/test/CIR/CodeGenOpenACC/combined-reduction-clause-default-ops.cpp
+++ b/clang/test/CIR/CodeGenOpenACC/combined-reduction-clause-default-ops.cpp
@@ -8,12 +8,19 @@ struct DefaultOperators {
bool b;
};
+struct DefaultOperatorsNoFloats {
+ int i;
+ unsigned int u;
+ bool b;
+};
+
template<typename T>
void acc_combined() {
T someVar;
T someVarArr[5];
+ struct DefaultOperatorsNoFloats someVarNoFloats;
+ struct DefaultOperatorsNoFloats someVarArrNoFloats[5];
#pragma acc parallel loop reduction(+:someVar)
- for(int i=0;i < 5; ++i);
// CHECK: acc.reduction.recipe @reduction_add__ZTS16DefaultOperators : !cir.ptr<!rec_DefaultOperators> reduction_operator <add> init {
// CHECK-NEXT: ^bb0(%[[ARG:.*]]: !cir.ptr<!rec_DefaultOperators>{{.*}})
// CHECK-NEXT: %[[ALLOCA:.*]] = cir.alloca !rec_DefaultOperators, !cir.ptr<!rec_DefaultOperators>, ["openacc.reduction.init", init]
@@ -39,6 +46,7 @@ void acc_combined() {
// TODO OpenACC: Expecting combination operation here
// CHECK-NEXT: acc.yield %[[LHSARG]] : !cir.ptr<!rec_DefaultOperators>
// CHECK-NEXT: }
+ for(int i=0;i < 5; ++i);
#pragma acc parallel loop reduction(*:someVar)
// CHECK-NEXT: acc.reduction.recipe @reduction_mul__ZTS16DefaultOperators : !cir.ptr<!rec_DefaultOperators> reduction_operator <mul> init {
@@ -121,86 +129,67 @@ void acc_combined() {
// CHECK-NEXT: acc.yield %[[LHSARG]] : !cir.ptr<!rec_DefaultOperators>
// CHECK-NEXT: }
for(int i=0;i < 5; ++i);
-#pragma acc parallel loop reduction(&:someVar)
-
-// CHECK-NEXT: acc.reduction.recipe @reduction_iand__ZTS16DefaultOperators : !cir.ptr<!rec_DefaultOperators> reduction_operator <iand> init {
-// CHECK-NEXT: ^bb0(%[[ARG:.*]]: !cir.ptr<!rec_DefaultOperators>{{.*}})
-// CHECK-NEXT: %[[ALLOCA:.*]] = cir.alloca !rec_DefaultOperators, !cir.ptr<!rec_DefaultOperators>, ["openacc.reduction.init", init]
-// CHECK-NEXT: %[[GET_I:.*]] = cir.get_member %[[ALLOCA]][0] {name = "i"} : !cir.ptr<!rec_DefaultOperators> -> !cir.ptr<!s32i>
+#pragma acc parallel loop reduction(&:someVarNoFloats)
+// CHECK-NEXT: acc.reduction.recipe @reduction_iand__ZTS24DefaultOperatorsNoFloats : !cir.ptr<!rec_DefaultOperatorsNoFloats> reduction_operator <iand> init {
+// CHECK-NEXT: ^bb0(%[[ARG:.*]]: !cir.ptr<!rec_DefaultOperatorsNoFloats>{{.*}})
+// CHECK-NEXT: %[[ALLOCA:.*]] = cir.alloca !rec_DefaultOperatorsNoFloats, !cir.ptr<!rec_DefaultOperatorsNoFloats>, ["openacc.reduction.init", init]
+// CHECK-NEXT: %[[GET_I:.*]] = cir.get_member %[[ALLOCA]][0] {name = "i"} : !cir.ptr<!rec_DefaultOperatorsNoFloats> -> !cir.ptr<!s32i>
// CHECK-NEXT: %[[ALL_ONES:.*]] = cir.const #cir.int<-1> : !s32i
// CHECK-NEXT: cir.store {{.*}} %[[ALL_ONES]], %[[GET_I]] : !s32i, !cir.ptr<!s32i>
-// CHECK-NEXT: %[[GET_U:.*]] = cir.get_member %[[ALLOCA]][1] {name = "u"} : !cir.ptr<!rec_DefaultOperators> -> !cir.ptr<!u32i>
+// CHECK-NEXT: %[[GET_U:.*]] = cir.get_member %[[ALLOCA]][1] {name = "u"} : !cir.ptr<!rec_DefaultOperatorsNoFloats> -> !cir.ptr<!u32i>
// CHECK-NEXT: %[[ALL_ONES:.*]] = cir.const #cir.int<4294967295> : !u32i
// CHECK-NEXT: cir.store {{.*}} %[[ALL_ONES]], %[[GET_U]] : !u32i, !cir.ptr<!u32i>
-// CHECK-NEXT: %[[GET_F:.*]] = cir.get_member %[[ALLOCA]][2] {name = "f"} : !cir.ptr<!rec_DefaultOperators> -> !cir.ptr<!cir.float>
-// CHECK-NEXT: %[[ALL_ONES:.*]] = cir.const #cir.fp<0xFF{{.*}}> : !cir.float
-// CHECK-NEXT: cir.store {{.*}} %[[ALL_ONES]], %[[GET_F]] : !cir.float, !cir.ptr<!cir.float>
-// CHECK-NEXT: %[[GET_D:.*]] = cir.get_member %[[ALLOCA]][3] {name = "d"} : !cir.ptr<!rec_DefaultOperators> -> !cir.ptr<!cir.double>
-// CHECK-NEXT: %[[ALL_ONES:.*]] = cir.const #cir.fp<0xFF{{.*}}> : !cir.double
-// CHECK-NEXT: cir.store {{.*}} %[[ALL_ONES]], %[[GET_D]] : !cir.double, !cir.ptr<!cir.double>
-// CHECK-NEXT: %[[GET_B:.*]] = cir.get_member %[[ALLOCA]][4] {name = "b"} : !cir.ptr<!rec_DefaultOperators> -> !cir.ptr<!cir.bool>
+// CHECK-NEXT: %[[GET_B:.*]] = cir.get_member %[[ALLOCA]][2] {name = "b"} : !cir.ptr<!rec_DefaultOperatorsNoFloats> -> !cir.ptr<!cir.bool>
// CHECK-NEXT: %[[ALL_ONES:.*]] = cir.const #true
// CHECK-NEXT: cir.store {{.*}} %[[ALL_ONES]], %[[GET_B]] : !cir.bool, !cir.ptr<!cir.bool>
// CHECK-NEXT: acc.yield
//
// CHECK-NEXT: } combiner {
-// CHECK-NEXT: ^bb0(%[[LHSARG:.*]]: !cir.ptr<!rec_DefaultOperators> {{.*}}, %[[RHSARG:.*]]: !cir.ptr<!rec_DefaultOperators> {{.*}})
+// CHECK-NEXT: ^bb0(%[[LHSARG:.*]]: !cir.ptr<!rec_DefaultOperatorsNoFloats> {{.*}}, %[[RHSARG:.*]]: !cir.ptr<!rec_DefaultOperatorsNoFloats> {{.*}})
// TODO OpenACC: Expecting combination operation here
-// CHECK-NEXT: acc.yield %[[LHSARG]] : !cir.ptr<!rec_DefaultOperators>
+// CHECK-NEXT: acc.yield %[[LHSARG]] : !cir.ptr<!rec_DefaultOperatorsNoFloats>
// CHECK-NEXT: }
- for(int i=0;i < 5; ++i);
-#pragma acc parallel loop reduction(|:someVar)
-// CHECK-NEXT: acc.reduction.recipe @reduction_ior__ZTS16DefaultOperators : !cir.ptr<!rec_DefaultOperators> reduction_operator <ior> init {
-// CHECK-NEXT: ^bb0(%[[ARG:.*]]: !cir.ptr<!rec_DefaultOperators>{{.*}})
-// CHECK-NEXT: %[[ALLOCA:.*]] = cir.alloca !rec_DefaultOperators, !cir.ptr<!rec_DefaultOperators>, ["openacc.reduction.init", init]
-// CHECK-NEXT: %[[GET_I:.*]] = cir.get_member %[[ALLOCA]][0] {name = "i"} : !cir.ptr<!rec_DefaultOperators> -> !cir.ptr<!s32i>
+ for(int i = 0; i < 5; ++i);
+#pragma acc parallel loop reduction(|:someVarNoFloats)
+// CHECK-NEXT: acc.reduction.recipe @reduction_ior__ZTS24DefaultOperatorsNoFloats : !cir.ptr<!rec_DefaultOperatorsNoFloats> reduction_operator <ior> init {
+// CHECK-NEXT: ^bb0(%[[ARG:.*]]: !cir.ptr<!rec_DefaultOperatorsNoFloats>{{.*}})
+// CHECK-NEXT: %[[ALLOCA:.*]] = cir.alloca !rec_DefaultOperatorsNoFloats, !cir.ptr<!rec_DefaultOperatorsNoFloats>, ["openacc.reduction.init", init]
+// CHECK-NEXT: %[[GET_I:.*]] = cir.get_member %[[ALLOCA]][0] {name = "i"} : !cir.ptr<!rec_DefaultOperatorsNoFloats> -> !cir.ptr<!s32i>
// CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !s32i
// CHECK-NEXT: cir.store {{.*}} %[[ZERO]], %[[GET_I]] : !s32i, !cir.ptr<!s32i>
-// CHECK-NEXT: %[[GET_U:.*]] = cir.get_member %[[ALLOCA]][1] {name = "u"} : !cir.ptr<!rec_DefaultOperators> -> !cir.ptr<!u32i>
+// CHECK-NEXT: %[[GET_U:.*]] = cir.get_member %[[ALLOCA]][1] {name = "u"} : !cir.ptr<!rec_DefaultOperatorsNoFloats> -> !cir.ptr<!u32i>
// CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !u32i
// CHECK-NEXT: cir.store {{.*}} %[[ZERO]], %[[GET_U]] : !u32i, !cir.ptr<!u32i>
-// CHECK-NEXT: %[[GET_F:.*]] = cir.get_member %[[ALLOCA]][2] {name = "f"} : !cir.ptr<!rec_DefaultOperators> -> !cir.ptr<!cir.float>
-// CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.fp<0{{.*}}> : !cir.float
-// CHECK-NEXT: cir.store {{.*}} %[[ZERO]], %[[GET_F]] : !cir.float, !cir.ptr<!cir.float>
-// CHECK-NEXT: %[[GET_D:.*]] = cir.get_member %[[ALLOCA]][3] {name = "d"} : !cir.ptr<!rec_DefaultOperators> -> !cir.ptr<!cir.double>
-// CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.fp<0{{.*}}> : !cir.double
-// CHECK-NEXT: cir.store {{.*}} %[[ZERO]], %[[GET_D]] : !cir.double, !cir.ptr<!cir.double>
-// CHECK-NEXT: %[[GET_B:.*]] = cir.get_member %[[ALLOCA]][4] {name = "b"} : !cir.ptr<!rec_DefaultOperators> -> !cir.ptr<!cir.bool>
+// CHECK-NEXT: %[[GET_B:.*]] = cir.get_member %[[ALLOCA]][2] {name = "b"} : !cir.ptr<!rec_DefaultOperatorsNoFloats> -> !cir.ptr<!cir.bool>
// CHECK-NEXT: %[[ZERO:.*]] = cir.const #false
// CHECK-NEXT: cir.store {{.*}} %[[ZERO]], %[[GET_B]] : !cir.bool, !cir.ptr<!cir.bool>
// CHECK-NEXT: acc.yield
//
// CHECK-NEXT: } combiner {
-// CHECK-NEXT: ^bb0(%[[LHSARG:.*]]: !cir.ptr<!rec_DefaultOperators> {{.*}}, %[[RHSARG:.*]]: !cir.ptr<!rec_DefaultOperators> {{.*}})
+// CHECK-NEXT: ^bb0(%[[LHSARG:.*]]: !cir.ptr<!rec_DefaultOperatorsNoFloats> {{.*}}, %[[RHSARG:.*]]: !cir.ptr<!rec_DefaultOperatorsNoFloats> {{.*}})
// TODO OpenACC: Expecting combination operation here
-// CHECK-NEXT: acc.yield %[[LHSARG]] : !cir.ptr<!rec_DefaultOperators>
+// CHECK-NEXT: acc.yield %[[LHSARG]] : !cir.ptr<!rec_DefaultOperatorsNoFloats>
// CHECK-NEXT: }
- for(int i=0;i < 5; ++i);
-#pragma acc parallel loop reduction(^:someVar)
-// CHECK-NEXT: acc.reduction.recipe @reduction_xor__ZTS16DefaultOperators : !cir.ptr<!rec_DefaultOperators> reduction_operator <xor> init {
-// CHECK-NEXT: ^bb0(%[[ARG:.*]]: !cir.ptr<!rec_DefaultOperators>{{.*}})
-// CHECK-NEXT: %[[ALLOCA:.*]] = cir.alloca !rec_DefaultOperators, !cir.ptr<!rec_DefaultOperators>, ["openacc.reduction.init", init]
-// CHECK-NEXT: %[[GET_I:.*]] = cir.get_member %[[ALLOCA]][0] {name = "i"} : !cir.ptr<!rec_DefaultOperators> -> !cir.ptr<!s32i>
+ for(int i = 0; i < 5; ++i);
+#pragma acc parallel loop reduction(^:someVarNoFloats)
+// CHECK-NEXT: acc.reduction.recipe @reduction_xor__ZTS24DefaultOperatorsNoFloats : !cir.ptr<!rec_DefaultOperatorsNoFloats> reduction_operator <xor> init {
+// CHECK-NEXT: ^bb0(%[[ARG:.*]]: !cir.ptr<!rec_DefaultOperatorsNoFloats>{{.*}})
+// CHECK-NEXT: %[[ALLOCA:.*]] = cir.alloca !rec_DefaultOperatorsNoFloats, !cir.ptr<!rec_DefaultOperatorsNoFloats>, ["openacc.reduction.init", init]
+// CHECK-NEXT: %[[GET_I:.*]] = cir.get_member %[[ALLOCA]][0] {name = "i"} : !cir.ptr<!rec_DefaultOperatorsNoFloats> -> !cir.ptr<!s32i>
// CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !s32i
// CHECK-NEXT: cir.store {{.*}} %[[ZERO]], %[[GET_I]] : !s32i, !cir.ptr<!s32i>
-// CHECK-NEXT: %[[GET_U:.*]] = cir.get_member %[[ALLOCA]][1] {name = "u"} : !cir.ptr<!rec_DefaultOperators> -> !cir.ptr<!u32i>
+// CHECK-NEXT: %[[GET_U:.*]] = cir.get_member %[[ALLOCA]][1] {name = "u"} : !cir.ptr<!rec_DefaultOperatorsNoFloats> -> !cir.ptr<!u32i>
// CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !u32i
// CHECK-NEXT: cir.store {{.*}} %[[ZERO]], %[[GET_U]] : !u32i, !cir.ptr<!u32i>
-// CHECK-NEXT: %[[GET_F:.*]] = cir.get_member %[[ALLOCA]][2] {name = "f"} : !cir.ptr<!rec_DefaultOperators> -> !cir.ptr<!cir.float>
-// CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.fp<0{{.*}}> : !cir.float
-// CHECK-NEXT: cir.store {{.*}} %[[ZERO]], %[[GET_F]] : !cir.float, !cir.ptr<!cir.float...
[truncated]
|
|
@thurstond : This is the re-apply. I'm not able to reproduce the issue but I am about 95% confident that this implementation will 'fix' the ASAn problem. I'll do my best to keep an eye on the same build-bots, but please let me know if you see a problem! |
Hmm.... I see here that it didn't: https://lab.llvm.org/buildbot/#/builders/55/builds/18562/steps/12/logs/stdio Though I'm not sure how much sense that makes, it seems to think that the 'uninitialized' memory from trailing storage actually had something in it that leaked (the stuff that got constructed over), which it obviously wasn't. I'm going to have to figure out a different layout or something for this... Working on it still. |
Oh wait, I am misunderstanding the output of ASAN. I know how to fix this now, urgh, I'm a silly one :) |
|
Thanks for the updates and for being responsive! Btw if you're having trouble with the buildbot scripts locally, I'd be happy to test your candidate patches with the buildbot script on my machine. |
Thanks! I have a patch I just merged that I think solves the last problem (a289e2f), and I THINK I am properly reproducing it locally (at least getting more information, though validating my patch doesn't really seem to work right?). So I merged it and am trying to validate locally as well as will keep track of the AArch64 buildbot. Else, I'll revert everything and try to do better tomorrow. Thanks for your help! |
The x86 asan buildbot is down at the moment, but I tried a289e2f locally with the asan buildbot script and it's passing. Thanks for fixing it! |
Thank you so much for confirming! It passed locally for me, and https://lab.llvm.org/buildbot/#/builders/55/builds/18579 seems to be succeeding (it at least didn't report anything there!). I believe the 'libcxx virtual function' diagnostic is going to be some sort of issue with the sanitizer itself, and is a biproduct of the sanitizer failing for a valid reason. So the |
akadutta
pushed a commit
to akadutta/llvm-project
that referenced
this pull request
Oct 14, 2025
llvm#163205) …(… (llvm#162920) This reverts commit 8d9aecc. That was reverted because of an ASAN failure that I believe this fixes. The std::uninitialized_copy of an array of a type that contained a llvm::vector using operator= instead of the copy constructor, so it made ASan think it was overwriting existing data? However, it also probably would have tried to do some sort of deallocation if had the 'right' amount of wrong data. So this version switches over to using placement-new instead.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…(… (#162920)
This reverts commit 8d9aecc.
That was reverted because of an ASAN failure that I believe this fixes. The std::uninitialized_copy of an array of a type that contained a llvm::vector using operator= instead of the copy constructor, so it made ASan think it was overwriting existing data? However, it also probably would have tried to do some sort of deallocation if had the 'right' amount of wrong data.
So this version switches over to using placement-new instead.