Skip to content

Commit b6dadc7

Browse files
authored
Revert "[OpenMP] Fix firstprivate pointer handling in target regions" (#169143)
Reverts #167879 This PR is causing assertions in the check-offload tests: https://lab.llvm.org/staging/#/builders/105 https://lab.llvm.org/staging/#/builders/105/builds/37057
1 parent 8bdbc57 commit b6dadc7

18 files changed

+34
-256
lines changed

clang/lib/CodeGen/CGOpenMPRuntime.cpp

Lines changed: 12 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
#include "clang/Basic/SourceManager.h"
2929
#include "clang/CodeGen/ConstantInitBuilder.h"
3030
#include "llvm/ADT/ArrayRef.h"
31-
#include "llvm/ADT/SmallSet.h"
3231
#include "llvm/ADT/SmallVector.h"
3332
#include "llvm/ADT/StringExtras.h"
3433
#include "llvm/Bitcode/BitcodeReader.h"
@@ -7212,9 +7211,6 @@ class MappableExprsHandler {
72127211
/// firstprivate, false otherwise.
72137212
llvm::DenseMap<CanonicalDeclPtr<const VarDecl>, bool> FirstPrivateDecls;
72147213

7215-
/// Set of defaultmap clause kinds that use firstprivate behavior.
7216-
llvm::SmallSet<OpenMPDefaultmapClauseKind, 4> DefaultmapFirstprivateKinds;
7217-
72187214
/// Map between device pointer declarations and their expression components.
72197215
/// The key value for declarations in 'this' is null.
72207216
llvm::DenseMap<
@@ -8993,10 +8989,6 @@ class MappableExprsHandler {
89938989
FirstPrivateDecls.try_emplace(VD, /*Implicit=*/true);
89948990
}
89958991
}
8996-
// Extract defaultmap clause information.
8997-
for (const auto *C : Dir.getClausesOfKind<OMPDefaultmapClause>())
8998-
if (C->getDefaultmapModifier() == OMPC_DEFAULTMAP_MODIFIER_firstprivate)
8999-
DefaultmapFirstprivateKinds.insert(C->getDefaultmapKind());
90008992
// Extract device pointer clause information.
90018993
for (const auto *C : Dir.getClausesOfKind<OMPIsDevicePtrClause>())
90028994
for (auto L : C->component_lists())
@@ -9574,35 +9566,6 @@ class MappableExprsHandler {
95749566
}
95759567
}
95769568

9577-
/// Check if a variable should be treated as firstprivate due to explicit
9578-
/// firstprivate clause or defaultmap(firstprivate:...).
9579-
bool isEffectivelyFirstprivate(const VarDecl *VD, QualType Type) const {
9580-
// Check explicit firstprivate clauses
9581-
if (FirstPrivateDecls.count(VD))
9582-
return true;
9583-
9584-
// Check defaultmap(firstprivate:scalar) for scalar types
9585-
if (DefaultmapFirstprivateKinds.count(OMPC_DEFAULTMAP_scalar)) {
9586-
if (Type->isScalarType())
9587-
return true;
9588-
}
9589-
9590-
// Check defaultmap(firstprivate:pointer) for pointer types
9591-
if (DefaultmapFirstprivateKinds.count(OMPC_DEFAULTMAP_pointer)) {
9592-
if (Type->isAnyPointerType())
9593-
return true;
9594-
}
9595-
9596-
// Check defaultmap(firstprivate:aggregate) for aggregate types
9597-
if (DefaultmapFirstprivateKinds.count(OMPC_DEFAULTMAP_aggregate)) {
9598-
if (Type->isAggregateType())
9599-
return true;
9600-
}
9601-
9602-
// Check defaultmap(firstprivate:all) for all types
9603-
return DefaultmapFirstprivateKinds.count(OMPC_DEFAULTMAP_all);
9604-
}
9605-
96069569
/// Generate the default map information for a given capture \a CI,
96079570
/// record field declaration \a RI and captured value \a CV.
96089571
void generateDefaultMapInfo(const CapturedStmt::Capture &CI,
@@ -9630,23 +9593,13 @@ class MappableExprsHandler {
96309593
CombinedInfo.DevicePtrDecls.push_back(nullptr);
96319594
CombinedInfo.DevicePointers.push_back(DeviceInfoTy::None);
96329595
CombinedInfo.Pointers.push_back(CV);
9633-
bool IsFirstprivate =
9634-
isEffectivelyFirstprivate(VD, RI.getType().getNonReferenceType());
9635-
96369596
if (!RI.getType()->isAnyPointerType()) {
96379597
// We have to signal to the runtime captures passed by value that are
96389598
// not pointers.
96399599
CombinedInfo.Types.push_back(
96409600
OpenMPOffloadMappingFlags::OMP_MAP_LITERAL);
96419601
CombinedInfo.Sizes.push_back(CGF.Builder.CreateIntCast(
96429602
CGF.getTypeSize(RI.getType()), CGF.Int64Ty, /*isSigned=*/true));
9643-
} else if (IsFirstprivate) {
9644-
// Firstprivate pointers should be passed by value (as literals)
9645-
// without performing a present table lookup at runtime.
9646-
CombinedInfo.Types.push_back(
9647-
OpenMPOffloadMappingFlags::OMP_MAP_LITERAL);
9648-
// Use zero size for pointer literals (just passing the pointer value)
9649-
CombinedInfo.Sizes.push_back(llvm::Constant::getNullValue(CGF.Int64Ty));
96509603
} else {
96519604
// Pointers are implicitly mapped with a zero size and no flags
96529605
// (other than first map that is added for all implicit maps).
@@ -9660,31 +9613,26 @@ class MappableExprsHandler {
96609613
assert(CI.capturesVariable() && "Expected captured reference.");
96619614
const auto *PtrTy = cast<ReferenceType>(RI.getType().getTypePtr());
96629615
QualType ElementType = PtrTy->getPointeeType();
9616+
CombinedInfo.Sizes.push_back(CGF.Builder.CreateIntCast(
9617+
CGF.getTypeSize(ElementType), CGF.Int64Ty, /*isSigned=*/true));
9618+
// The default map type for a scalar/complex type is 'to' because by
9619+
// default the value doesn't have to be retrieved. For an aggregate
9620+
// type, the default is 'tofrom'.
9621+
CombinedInfo.Types.push_back(getMapModifiersForPrivateClauses(CI));
96639622
const VarDecl *VD = CI.getCapturedVar();
9664-
bool IsFirstprivate = isEffectivelyFirstprivate(VD, ElementType);
9623+
auto I = FirstPrivateDecls.find(VD);
96659624
CombinedInfo.Exprs.push_back(VD->getCanonicalDecl());
96669625
CombinedInfo.BasePointers.push_back(CV);
96679626
CombinedInfo.DevicePtrDecls.push_back(nullptr);
96689627
CombinedInfo.DevicePointers.push_back(DeviceInfoTy::None);
9669-
9670-
// For firstprivate pointers, pass by value instead of dereferencing
9671-
if (IsFirstprivate && ElementType->isAnyPointerType()) {
9672-
// Treat as a literal value (pass the pointer value itself)
9673-
CombinedInfo.Pointers.push_back(CV);
9674-
// Use zero size for pointer literals
9675-
CombinedInfo.Sizes.push_back(llvm::Constant::getNullValue(CGF.Int64Ty));
9676-
CombinedInfo.Types.push_back(
9677-
OpenMPOffloadMappingFlags::OMP_MAP_LITERAL);
9628+
if (I != FirstPrivateDecls.end() && ElementType->isAnyPointerType()) {
9629+
Address PtrAddr = CGF.EmitLoadOfReference(CGF.MakeAddrLValue(
9630+
CV, ElementType, CGF.getContext().getDeclAlign(VD),
9631+
AlignmentSource::Decl));
9632+
CombinedInfo.Pointers.push_back(PtrAddr.emitRawPointer(CGF));
96789633
} else {
9679-
CombinedInfo.Sizes.push_back(CGF.Builder.CreateIntCast(
9680-
CGF.getTypeSize(ElementType), CGF.Int64Ty, /*isSigned=*/true));
9681-
// The default map type for a scalar/complex type is 'to' because by
9682-
// default the value doesn't have to be retrieved. For an aggregate
9683-
// type, the default is 'tofrom'.
9684-
CombinedInfo.Types.push_back(getMapModifiersForPrivateClauses(CI));
96859634
CombinedInfo.Pointers.push_back(CV);
96869635
}
9687-
auto I = FirstPrivateDecls.find(VD);
96889636
if (I != FirstPrivateDecls.end())
96899637
IsImplicit = I->getSecond();
96909638
}

clang/test/OpenMP/target_codegen.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@
7878
// code and have mapped arguments, and only 6 have all-constant map sizes.
7979

8080
// CHECK-DAG: [[SIZET:@.+]] = private unnamed_addr constant [2 x i64] [i64 0, i64 4]
81-
// CHECK-DAG: [[MAPT:@.+]] = private unnamed_addr constant [2 x i64] [i64 800, i64 800]
81+
// CHECK-DAG: [[MAPT:@.+]] = private unnamed_addr constant [2 x i64] [i64 544, i64 800]
8282
// CHECK-DAG: [[SIZET2:@.+]] = private unnamed_addr constant [1 x i{{32|64}}] [i64 2]
8383
// CHECK-DAG: [[MAPT2:@.+]] = private unnamed_addr constant [1 x i64] [i64 800]
8484
// CHECK-DAG: [[SIZET3:@.+]] = private unnamed_addr constant [2 x i64] [i64 4, i64 2]

clang/test/OpenMP/target_defaultmap_codegen_01.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -735,7 +735,7 @@ void explicit_maps_single (){
735735

736736
// CK14: [[SIZE09:@.+]] = private {{.*}}constant [1 x i64] zeroinitializer
737737
// Map types: OMP_MAP_TARGET_PARAM | OMP_MAP_IMPLICIT = 544
738-
// CK14: [[MTYPE09:@.+]] = private {{.*}}constant [1 x i64] [i64 800]
738+
// CK14: [[MTYPE09:@.+]] = private {{.*}}constant [1 x i64] [i64 544]
739739

740740
// CK14-LABEL: explicit_maps_single{{.*}}(
741741
void explicit_maps_single (){
@@ -1235,7 +1235,7 @@ void implicit_maps_struct (int a){
12351235

12361236
// CK22-DAG: [[SIZES:@.+]] = {{.+}}constant [1 x i64] zeroinitializer
12371237
// Map types: OMP_MAP_TARGET_PARAM | OMP_MAP_IMPLICIT = 544
1238-
// CK22-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i64] [i64 800]
1238+
// CK22-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i64] [i64 544]
12391239

12401240
// CK22-LABEL: implicit_maps_pointer{{.*}}(
12411241
void implicit_maps_pointer (){

clang/test/OpenMP/target_depend_codegen.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
// TCHECK: [[ENTTY:%.+]] = type { i64, i16, i16, i32, ptr, ptr, i64, i64, ptr }
4545

4646
// CHECK-DAG: [[SIZET:@.+]] = private unnamed_addr constant [3 x i64] [i64 0, i64 4, i64 {{16|12}}]
47-
// CHECK-DAG: [[MAPT:@.+]] = private unnamed_addr constant [3 x i64] [i64 800, i64 800, i64 3]
47+
// CHECK-DAG: [[MAPT:@.+]] = private unnamed_addr constant [3 x i64] [i64 544, i64 800, i64 3]
4848
// CHECK-DAG: @{{.*}} = weak constant i8 0
4949

5050
// TCHECK: @{{.+}} = weak constant [[ENTTY]]

clang/test/OpenMP/target_firstprivate_pointer_codegen.cpp

Lines changed: 0 additions & 169 deletions
This file was deleted.

clang/test/OpenMP/target_map_codegen_01.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,12 @@
3838
// CK2-LABEL: @.__omp_offloading_{{.*}}implicit_maps_reference{{.*}}_l{{[0-9]+}}.region_id = weak constant i8 0
3939

4040
// CK2: [[SIZES:@.+]] = {{.+}}constant [1 x i64] [i64 4]
41-
// Map types: OMP_MAP_LITERAL | OMP_MAP_TARGET_PARAM | OMP_MAP_IMPLICIT = 800
41+
// Map types: OMP_MAP_PRIVATE_VAL | OMP_MAP_TARGET_PARAM | OMP_MAP_IMPLICIT = 800
4242
// CK2: [[TYPES:@.+]] = {{.+}}constant [1 x i64] [i64 800]
4343
// CK2-LABEL: @.__omp_offloading_{{.*}}implicit_maps_reference{{.*}}_l{{[0-9]+}}.region_id = weak constant i8 0
4444
// CK2: [[SIZES2:@.+]] = {{.+}}constant [1 x i64] zeroinitializer
45-
// Map types: OMP_MAP_IS_PTR | OMP_MAP_LITERAL | OMP_MAP_TARGET_PARAM | OMP_MAP_IMPLICIT = 800
46-
// CK2: [[TYPES2:@.+]] = {{.+}}constant [1 x i64] [i64 800]
45+
// Map types: OMP_MAP_IS_PTR | OMP_MAP_IMPLICIT = 544
46+
// CK2: [[TYPES2:@.+]] = {{.+}}constant [1 x i64] [i64 544]
4747

4848
// CK2-LABEL: implicit_maps_reference{{.*}}(
4949
void implicit_maps_reference (int a, int *b){

clang/test/OpenMP/target_map_codegen_09.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@
3636
// CK10-LABEL: @.__omp_offloading_{{.*}}implicit_maps_pointer{{.*}}_l{{[0-9]+}}.region_id = weak constant i8 0
3737

3838
// CK10-DAG: [[SIZES:@.+]] = {{.+}}constant [1 x i64] zeroinitializer
39-
// Map types: OMP_MAP_IS_PTR | OMP_MAP_LITERAL | OMP_MAP_TARGET_PARAM | OMP_MAP_IMPLICIT = 800
40-
// CK10-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i64] [i64 800]
39+
// Map types: OMP_MAP_TARGET_PARAM | OMP_MAP_IMPLICIT = 544
40+
// CK10-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i64] [i64 544]
4141

4242
// CK10-LABEL: implicit_maps_pointer{{.*}}(
4343
void implicit_maps_pointer (){

clang/test/OpenMP/target_map_codegen_10.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,7 @@
3232
// CK11_5-DAG: [[SIZES:@.+]] = {{.+}}constant [2 x i64] [i64 16, i64 0]
3333
// Map types: OMP_MAP_TO | OMP_MAP_FROM | OMP_MAP_TARGET_PARAM | OMP_MAP_IMPLICIT = 547
3434
// CK11_4-DAG: [[TYPES:@.+]] = {{.+}}constant [2 x i64] [i64 547, i64 547]
35-
// Map types: OMP_MAP_TO | OMP_MAP_FROM | OMP_MAP_TARGET_PARAM | OMP_MAP_IMPLICIT = 547, OMP_MAP_IS_PTR | OMP_MAP_LITERAL | OMP_MAP_TARGET_PARAM | OMP_MAP_IMPLICIT = 800
36-
// CK11_5-DAG: [[TYPES:@.+]] = {{.+}}constant [2 x i64] [i64 547, i64 800]
35+
// CK11_5-DAG: [[TYPES:@.+]] = {{.+}}constant [2 x i64] [i64 547, i64 544]
3736

3837
// CK11-LABEL: implicit_maps_double_complex{{.*}}(
3938
void implicit_maps_double_complex (int a, int *b){

clang/test/OpenMP/target_map_codegen_26.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535

3636
// CK27-LABEL: @.__omp_offloading_{{.*}}zero_size_section_and_private_maps{{.*}}_l{{[0-9]+}}.region_id = weak constant i8 0
3737
// CK27: [[SIZE00:@.+]] = private {{.*}}constant [1 x i64] zeroinitializer
38-
// CK27: [[MTYPE00:@.+]] = private {{.*}}constant [1 x i64] [i64 800]
38+
// CK27: [[MTYPE00:@.+]] = private {{.*}}constant [1 x i64] [i64 544]
3939

4040
// CK27-LABEL: @.__omp_offloading_{{.*}}zero_size_section_and_private_maps{{.*}}_l{{[0-9]+}}.region_id = weak constant i8 0
4141
// CK27: [[SIZE01:@.+]] = private {{.*}}constant [1 x i64] zeroinitializer
@@ -52,7 +52,7 @@
5252
// CK27-LABEL: @.__omp_offloading_{{.*}}zero_size_section_and_private_maps{{.*}}_l{{[0-9]+}}.region_id = weak constant i8 0
5353
// CK27-LABEL: @.__omp_offloading_{{.*}}zero_size_section_and_private_maps{{.*}}_l{{[0-9]+}}.region_id = weak constant i8 0
5454
// CK27: [[SIZE05:@.+]] = private {{.*}}constant [1 x i64] zeroinitializer
55-
// CK27: [[MTYPE05:@.+]] = private {{.*}}constant [1 x i64] [i64 288]
55+
// CK27: [[MTYPE05:@.+]] = private {{.*}}constant [1 x i64] [i64 32]
5656

5757
// CK27-LABEL: @.__omp_offloading_{{.*}}zero_size_section_and_private_maps{{.*}}_l{{[0-9]+}}.region_id = weak constant i8 0
5858
// CK27-LABEL: @.__omp_offloading_{{.*}}zero_size_section_and_private_maps{{.*}}_l{{[0-9]+}}.region_id = weak constant i8 0

0 commit comments

Comments
 (0)