[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

jdoerfert added a comment. In D71475#1791825 , @cchen wrote: > [...] I found it really hard to update the tests by reading the diagnostic > message. Can I refactor the test a bit (like separate each openmp portion to > be CK1, CK2...) so that I can

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

cchen added a comment. In D71475#1788187 , @ABataev wrote: > The fixed patch. Several codegen tests require some adjustment. @Abataev, thanks for the reply. However, for adjusting the codegen, I found it really hard to update the tests by reading the

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

ABataev added a comment. The fixed patch. Several codegen tests require some adjustment. diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index afe0f1a..ecb0fb2 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -919,6 +919,11 @@

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1128 + (A == OMPC_firstprivate && (Data.Attributes == OMPC_lastprivate || + Data.Attributes == OMPC_linear)) || (A == OMPC_lastprivate &&

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

jdoerfert added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1128 + (A == OMPC_firstprivate && (Data.Attributes == OMPC_lastprivate || + Data.Attributes == OMPC_linear)) || (A == OMPC_lastprivate &&

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

cchen added a comment. In D71475#1787914 , @ABataev wrote: > Here is more proper fix. We don't need to capture just `k` here, instead, we > need to capture the whole expression. > Linear clause has a little bit different processing rather than all other

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1128 + (A == OMPC_firstprivate && (Data.Attributes == OMPC_lastprivate || + Data.Attributes == OMPC_linear)) || (A == OMPC_lastprivate &&

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

jdoerfert added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1128 + (A == OMPC_firstprivate && (Data.Attributes == OMPC_lastprivate || + Data.Attributes == OMPC_linear)) || (A == OMPC_lastprivate &&

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1128 + (A == OMPC_firstprivate && (Data.Attributes == OMPC_lastprivate || + Data.Attributes == OMPC_linear)) || (A == OMPC_lastprivate &&

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1128 + (A == OMPC_firstprivate && (Data.Attributes == OMPC_lastprivate || + Data.Attributes == OMPC_linear)) ||

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

ABataev added a comment. Here is more proper fix. We don't need to capture just `k` here, instead, we need to capture the whole expression. Linear clause has a little bit different processing rather than all other clauses caused by a non-perfect design. Add codegen tests for all combined

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

jdoerfert added a comment. In D71475#1784312 , @cchen wrote: > In D71475#1784284 , @jdoerfert wrote: > > > What is the output when you run the example with `k` in `lastprivate` or > > `reduction`? > > > I actually

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

cchen updated this revision to Diff 234157. cchen added a comment. Apply @ABataev's patch, add some code to support the patch and some tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71475/new/ https://reviews.llvm.org/D71475 Files:

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

cchen added a comment. > It will still crash for something like `target simd`, need to move the code > around a little bit, move it out of `if`. I tried making `target simd ` crash but it seems work (target_simd_ast_print.cpp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

ABataev added a comment. Here is the fix: diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index e02c1c5..5ce81b0 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -3830,6 +3830,9 @@ StmtResult

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

cchen marked an inline comment as done. cchen added a comment. In D71475#1784284 , @jdoerfert wrote: > What is the output when you run the example with `k` in `lastprivate` or > `reduction`? I actually got the same result (return value) changing from

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

jdoerfert added a comment. What is the output when you run the example with `k` in `lastprivate` or `reduction`? In D71475#1784173 , @cchen wrote: > Doing this still fail the assertion since we still don't have the variable > inside > CapturedStmt.

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

cchen updated this revision to Diff 233869. cchen added a comment. Remove debug code and some redundancy Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71475/new/ https://reviews.llvm.org/D71475 Files: clang/lib/Sema/SemaOpenMP.cpp

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

cchen updated this revision to Diff 233866. cchen added a comment. Add linear step var into Implicitfirstprivate Doing this still fail the assertion since we still don't have the variable inside CapturedStmt. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

jdoerfert added a comment. In D71475#1783928 , @cchen wrote: > In D71475#1783834 , @jdoerfert wrote: > > > Your commit message example lacks the #pragma. > > > > What if you add k to the list of explicit

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

cchen added a comment. In D71475#1783834 , @jdoerfert wrote: > Your commit message example lacks the #pragma. > > What if you add k to the list of explicit firstprivate? (I mean, you can try > it in C first). > > And how do I reproduce the crash? I

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

jdoerfert added a comment. Your commit message example lacks the #pragma. What if you add k to the list of explicit firstprivate? (I mean, you can try it in C first). And how do I reproduce the crash? I tried: https://godbolt.org/z/FDPSnA Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

cchen created this revision. cchen added a reviewer: ABataev. Herald added subscribers: cfe-commits, guansong. Herald added a reviewer: jdoerfert. Herald added a project: clang. For this example: int a[100]; int f2 (int i, int k) { for (int j = 16; j < 64; j++) {