Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for codegen Crash in Clang when using locator omp_all_memory with depobj construct #114221

Merged
merged 7 commits into from
Nov 11, 2024
Merged

Fix for codegen Crash in Clang when using locator omp_all_memory with depobj construct #114221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4e6d8c1
Fix for codegen Crash in Clang when using locator omp_all_memory with…
Oct 30, 2024
30eb825
clang-format fix
Nov 4, 2024
a35fe7e
updated with codegen checks
Nov 7, 2024
15a29e5
Merge branch 'main' into depobj_crash
chandraghale Nov 7, 2024
ae42197
code formatting
Nov 7, 2024
885be5b
Merge branch 'depobj_crash' of https://github.com/chandraghale/llvm-p…
Nov 7, 2024
8b5983f
Merge branch 'main' into depobj_crash
chandraghale Nov 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions clang/lib/CodeGen/CGStmtOpenMP.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5556,12 +5556,14 @@ void CodeGenFunction::EmitOMPDepobjDirective(const OMPDepobjDirective &S) {
const auto *DO = S.getSingleClause<OMPDepobjClause>();
LValue DOLVal = EmitLValue(DO->getDepobj());
if (const auto *DC = S.getSingleClause<OMPDependClause>()) {
OMPTaskDataTy::DependData Dependencies(DC->getDependencyKind(),
DC->getModifier());
Dependencies.DepExprs.append(DC->varlist_begin(), DC->varlist_end());
Address DepAddr = CGM.getOpenMPRuntime().emitDepobjDependClause(
*this, Dependencies, DC->getBeginLoc());
EmitStoreOfScalar(DepAddr.emitRawPointer(*this), DOLVal);
// Build list and emit dependences
OMPTaskDataTy Data;
buildDependences(S, Data);
for (auto &Dep : Data.Dependences) {
Address DepAddr = CGM.getOpenMPRuntime().emitDepobjDependClause(
*this, Dep, DC->getBeginLoc());
EmitStoreOfScalar(DepAddr.emitRawPointer(*this), DOLVal);
}
return;
}
if (const auto *DC = S.getSingleClause<OMPDestroyClause>()) {
Expand Down
32 changes: 30 additions & 2 deletions clang/test/OpenMP/depobj_codegen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@
typedef void *omp_depend_t;

void foo() {}
void tmainc(){
omp_depend_t obj;
#pragma omp depobj(obj) depend(inout: omp_all_memory)
{
volatile omp_depend_t temp = obj;
char* char_ptr = reinterpret_cast<char*>(temp);
char_ptr[0] = 1;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

even though we don't enforce code format in test, could you just do it manually?

#pragma omp depobj(obj) depend(inout: omp_all_memory)
  {
    ...
  }

Copy link
Contributor Author

@chandraghale chandraghale Nov 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shiltian Is this correct now for what you asked ?

}

template <class T>
T tmain(T argc) {
Expand All @@ -35,10 +44,25 @@ int main(int argc, char **argv) {
#pragma omp depobj(b) destroy
#pragma omp depobj(b) update(mutexinoutset)
#pragma omp depobj(a) depend(iterator(char *p = argv[argc]:argv[0]:-1), out: p[0])
(void)tmain(a), tmain(b);
(void)tmain(a), tmain(b);
tmainc();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here as well, at least not making it worse

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same !!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping @shiltian updated !!

return 0;
}

// CHECK-LABEL: tmainc
// CHECK: [[D_ADDR:%obj]] = alloca ptr,
// CHECK: [[GTID:%.+]] = call i32 @__kmpc_global_thread_num(
// CHECK: [[DEP_ADDR_ADDR2:%.+]] = call ptr @__kmpc_alloc(i32 [[GTID]], i64 48, ptr null)
// CHECK: [[SZ_DEOOBJ:%.+]] = getelementptr inbounds nuw %struct.kmp_depend_info, ptr [[DEP_ADDR_ADDR2]], i{{.+}} 0, i{{.+}} 0
// CHECK: store i64 1, ptr [[SZ_DEOOBJ]], align 8
// CHECK: [[DEPOBJ_BASE_ADDR:%.+]] = getelementptr %struct.kmp_depend_info, ptr [[DEP_ADDR_ADDR2]], i{{.+}} 1
// CHECK: [[ADDR_ONE:%.+]] = getelementptr inbounds nuw %struct.kmp_depend_info, ptr [[DEPOBJ_BASE_ADDR]], i{{.+}} 0, i{{.+}} 0
// CHECK: store i64 0, ptr [[ADDR_ONE]], align 8
// CHECK: [[SZ_ADDR:%.+]] = getelementptr inbounds nuw %struct.kmp_depend_info, ptr [[DEPOBJ_BASE_ADDR]], i{{.+}} 0, i{{.+}} 1
// CHECK: store i64 0, ptr [[SZ_ADDR]], align 8
// CHECK: [[SZ_ADDR_NEW:%.+]] = getelementptr inbounds nuw %struct.kmp_depend_info, ptr [[DEPOBJ_BASE_ADDR]], i{{.+}} 0, i{{.+}} 2
// CHECK: store {{i[0-9]+}} {{-?[0-9]+}}, ptr [[SZ_ADDR_NEW]], align 8
// CHECK: [[DEP_NEW:%.+]] = getelementptr %struct.kmp_depend_info, ptr [[DEP_ADDR_ADDR2]], i{{.+}} 1
// CHECK: store ptr [[DEP_NEW]], ptr [[D_ADDR]], align 8
// CHECK-LABEL: @main
// CHECK: [[B_ADDR:%b]] = alloca ptr,
// CHECK: [[GTID:%.+]] = call i32 @__kmpc_global_thread_num(
Expand Down Expand Up @@ -69,6 +93,8 @@ int main(int argc, char **argv) {
// CHECK: [[NUMDEPS_ADDR:%.+]] = getelementptr inbounds nuw %struct.kmp_depend_info, ptr [[NUMDEPS_BASE]], i{{.+}} 0, i{{.+}} 0
// CHECK: [[NUMDEPS:%.+]] = load i64, ptr [[NUMDEPS_ADDR]], align 8
// CHECK: [[END:%.+]] = getelementptr %struct.kmp_depend_info, ptr [[B_BASE]], i64 [[NUMDEPS]]


// CHECK: br label %[[BODY:.+]]
// CHECK: [[BODY]]:
// CHECK: [[EL:%.+]] = phi ptr [ [[B_BASE]], %{{.+}} ], [ [[EL_NEXT:%.+]], %[[BODY]] ]
Expand Down Expand Up @@ -228,6 +254,8 @@ int main(int argc, char **argv) {
// CHECK: [[EL_NEXT]] = getelementptr %struct.kmp_depend_info, ptr [[EL]], i{{.+}} 1
// CHECK: [[IS_DONE:%.+]] = icmp eq ptr [[EL_NEXT]], [[END]]
// CHECK: br i1 [[IS_DONE]], label %[[DONE:.+]], label %[[BODY]]

// CHECK: [[DONE]]:


#endif
Loading