[clang] d0d43b5 - [OpenMP] target nested `use_device_ptr() if()` and is_device_ptr trigger asserts
via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 4 10:37:08 PST 2020
Author: cchen
Date: 2020-11-04T12:36:57-06:00
New Revision: d0d43b58b109c2945e30d0bfabe77d3dcf1e4ad5
URL: https://github.com/llvm/llvm-project/commit/d0d43b58b109c2945e30d0bfabe77d3dcf1e4ad5
DIFF: https://github.com/llvm/llvm-project/commit/d0d43b58b109c2945e30d0bfabe77d3dcf1e4ad5.diff
LOG: [OpenMP] target nested `use_device_ptr() if()` and is_device_ptr trigger asserts
Clang now asserts for the below case:
```
void clang::CodeGen::CGOpenMPRuntime::createOffloadEntriesAndInfoMetadata(): Assertion `std::get<0>(E) && "All ordered entries must exist!"' failed.
```
The reason why Clang hit the assert is because in
`emitTargetDataCalls`, both `BeginThenGen` and `BeginElseGen` call
`registerTargetRegionEntryInfo` and try to register the Entry in
OffloadEntriesTargetRegion with same key. If changing the expression in
if clause to any constant expression, then the assert disappear. (https://godbolt.org/z/TW7haj)
The assert itself is to avoid
user from accessing elements out of bound inside `OrderedEntries` in
`createOffloadEntriesAndInfoMetadata`.
In this patch, I add a check in `registerTargetRegionEntryInfo` to avoid
register the target region more than once.
A test case that triggers assert: https://godbolt.org/z/4cnGW8
Reviewed By: ABataev
Differential Revision: https://reviews.llvm.org/D90704
Added:
clang/test/OpenMP/target_data_use_device_ptr_if_codegen.cpp
Modified:
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/CodeGen/CGOpenMPRuntime.h
Removed:
################################################################################
diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index f62b64795f8d..05a987bb04f1 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -2955,6 +2955,13 @@ void CGOpenMPRuntime::OffloadEntriesInfoManagerTy::
Entry.setID(ID);
Entry.setFlags(Flags);
} else {
+ if (Flags ==
+ OffloadEntriesInfoManagerTy::OMPTargetRegionEntryTargetRegion &&
+ hasTargetRegionEntryInfo(DeviceID, FileID, ParentName, LineNum,
+ /*IgnoreAddressId*/ true))
+ return;
+ assert(!hasTargetRegionEntryInfo(DeviceID, FileID, ParentName, LineNum) &&
+ "Target region entry already registered!");
OffloadEntryInfoTargetRegion Entry(OffloadingEntriesNum, Addr, ID, Flags);
OffloadEntriesTargetRegion[DeviceID][FileID][ParentName][LineNum] = Entry;
++OffloadingEntriesNum;
@@ -2962,8 +2969,8 @@ void CGOpenMPRuntime::OffloadEntriesInfoManagerTy::
}
bool CGOpenMPRuntime::OffloadEntriesInfoManagerTy::hasTargetRegionEntryInfo(
- unsigned DeviceID, unsigned FileID, StringRef ParentName,
- unsigned LineNum) const {
+ unsigned DeviceID, unsigned FileID, StringRef ParentName, unsigned LineNum,
+ bool IgnoreAddressId) const {
auto PerDevice = OffloadEntriesTargetRegion.find(DeviceID);
if (PerDevice == OffloadEntriesTargetRegion.end())
return false;
@@ -2977,7 +2984,8 @@ bool CGOpenMPRuntime::OffloadEntriesInfoManagerTy::hasTargetRegionEntryInfo(
if (PerLine == PerParentName->second.end())
return false;
// Fail if this entry is already registered.
- if (PerLine->second.getAddress() || PerLine->second.getID())
+ if (!IgnoreAddressId &&
+ (PerLine->second.getAddress() || PerLine->second.getID()))
return false;
return true;
}
diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.h b/clang/lib/CodeGen/CGOpenMPRuntime.h
index d67034cb3de6..8da028d06de8 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.h
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.h
@@ -613,7 +613,8 @@ class CGOpenMPRuntime {
/// Return true if a target region entry with the provided information
/// exists.
bool hasTargetRegionEntryInfo(unsigned DeviceID, unsigned FileID,
- StringRef ParentName, unsigned LineNum) const;
+ StringRef ParentName, unsigned LineNum,
+ bool IgnoreAddressId = false) const;
/// brief Applies action \a Action on all registered entries.
typedef llvm::function_ref<void(unsigned, unsigned, StringRef, unsigned,
const OffloadEntryInfoTargetRegion &)>
diff --git a/clang/test/OpenMP/target_data_use_device_ptr_if_codegen.cpp b/clang/test/OpenMP/target_data_use_device_ptr_if_codegen.cpp
new file mode 100644
index 000000000000..2a57497b5fd6
--- /dev/null
+++ b/clang/test/OpenMP/target_data_use_device_ptr_if_codegen.cpp
@@ -0,0 +1,48 @@
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+///==========================================================================///
+// RUN: %clang_cc1 -DCK1 -verify -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix CK1 --check-prefix CK1-64
+// RUN: %clang_cc1 -DCK1 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefix CK1 --check-prefix CK1-64
+// RUN: %clang_cc1 -DCK1 -verify -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix CK1 --check-prefix CK1-32
+// RUN: %clang_cc1 -DCK1 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefix CK1 --check-prefix CK1-32
+
+// RUN: %clang_cc1 -DCK1 -verify -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY1 %s
+// RUN: %clang_cc1 -DCK1 -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY1 %s
+// RUN: %clang_cc1 -DCK1 -verify -fopenmp-simd -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY1 %s
+// RUN: %clang_cc1 -DCK1 -fopenmp-simd -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY1 %s
+// SIMD-ONLY1-NOT: {{__kmpc|__tgt}}
+#ifdef CK1
+
+// CK1: [[MTYPE00:@.+]] = {{.*}}constant [1 x i64] [i64 99]
+// CK1: [[MTYPE01:@.+]] = {{.*}}constant [1 x i64] [i64 288]
+// CK1: [[MTYPE02:@.+]] = {{.*}}constant [1 x i64] [i64 288]
+
+void add_one(float *b, int dm)
+{
+ // CK1: [[BP:%.+]] = getelementptr inbounds [1 x i8*], [1 x i8*]* %{{.+}}, i32 0, i32 0
+ // CK1: [[CBP:%.+]] = bitcast i8** [[BP]] to float**
+ // CK1: store float* [[B_ADDR:%.+]], float** [[CBP]]
+ // CK1: call void @__tgt_target_data_begin{{.+}}[[MTYPE00]]
+ // CK1: [[VAL:%.+]] = load float*, float** [[CBP]],
+ // CK1-NOT: store float* [[VAL]], float** [[DECL]],
+ // CK1: store float* [[VAL]], float** [[PVT:%.+]],
+ // CK1: [[TT:%.+]] = load float*, float** [[PVT]],
+ // CK1: call i32 @__tgt_target{{.+}}[[MTYPE01]]
+ // CK1: call i32 @__tgt_target{{.+}}[[MTYPE02]]
+ // CK1: call void @__tgt_target_data_end{{.+}}[[MTYPE00]]
+#pragma omp target data map(tofrom:b[:1]) use_device_ptr(b) if(dm == 0)
+ {
+#pragma omp target is_device_ptr(b)
+ {
+ b[0] += 1;
+ }
+ }
+}
+
+#endif
+#endif
More information about the cfe-commits
mailing list