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

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

2019-12-19 Thread Johannes Doerfert via Phabricator via cfe-commits

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 modify the test easier.


This is a concern multiple people have raised already. Anything that helps 
making tests easier to read and update is good!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71475/new/

https://reviews.llvm.org/D71475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-12-19 Thread Chi Chun Chen via Phabricator via cfe-commits

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 diagnostic message. Can I 
refactor the test a bit (like separate each openmp portion to be CK1, CK2...) 
so that I can modify the test easier.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71475/new/

https://reviews.llvm.org/D71475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-12-17 Thread Alexey Bataev via Phabricator via cfe-commits

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 @@ static const Expr *getExprAsWritten(const Expr *E) {
  
 if (const auto *ICE = dyn_cast(E))
   E = ICE->getSubExprAsWritten();
  +
  +  if (const auto *DRE = dyn_cast(E))
  +if (const auto *CED = dyn_cast(DRE->getDecl()))
  +  E = getExprAsWritten(CED->getInit());
  +
 return E->IgnoreParens();
   }
  
  @@ -3830,6 +3835,9 @@ StmtResult Sema::ActOnOpenMPRegionEnd(StmtResult S,
 MarkDeclarationsReferencedInExpr(E);
   }
 }
  +  if (auto *LC = dyn_cast(Clause))
  +if (Expr *E = LC->getStep())
  +  MarkDeclarationsReferencedInExpr(E);
 DSAStack->setForceVarCapturing(/*V=*/false);
   } else if (CaptureRegions.size() > 1 ||
  CaptureRegions.back() != OMPD_unknown) {
  @@ -11396,12 +11404,88 @@ static OpenMPDirectiveKind 
getOpenMPCaptureRegionForClause(
 llvm_unreachable("Unknown OpenMP directive");
   }
   break;
  +  case OMPC_linear:
  +switch (DKind) {
  +case OMPD_taskloop_simd:
  +case OMPD_master_taskloop_simd:
  +case OMPD_parallel_master_taskloop_simd:
  +  CaptureRegion = OMPD_taskloop;
  +  break;
  +case OMPD_target_simd:
  +  CaptureRegion = OMPD_target;
  +  break;
  +case OMPD_target_teams_distribute_simd:
  +case OMPD_teams_distribute_simd:
  +  CaptureRegion = OMPD_teams;
  +  break;
  +case OMPD_target_parallel_for:
  +case OMPD_target_parallel_for_simd:
  +case OMPD_target_teams_distribute_parallel_for:
  +case OMPD_target_teams_distribute_parallel_for_simd:
  +case OMPD_teams_distribute_parallel_for:
  +case OMPD_teams_distribute_parallel_for_simd:
  +case OMPD_distribute_parallel_for:
  +case OMPD_distribute_parallel_for_simd:
  +case OMPD_parallel_for:
  +case OMPD_parallel_for_simd:
  +  CaptureRegion = OMPD_parallel;
  +  break;
  +case OMPD_simd:
  +case OMPD_for:
  +case OMPD_for_simd:
  +case OMPD_distribute_simd:
  +  break;
  +case OMPD_parallel_master_taskloop:
  +case OMPD_task:
  +case OMPD_taskloop:
  +case OMPD_master_taskloop:
  +case OMPD_target_update:
  +case OMPD_target_enter_data:
  +case OMPD_target_exit_data:
  +case OMPD_target:
  +case OMPD_target_teams:
  +case OMPD_target_parallel:
  +case OMPD_target_teams_distribute:
  +case OMPD_target_data:
  +case OMPD_teams:
  +case OMPD_teams_distribute:
  +case OMPD_cancel:
  +case OMPD_parallel:
  +case OMPD_parallel_master:
  +case OMPD_parallel_sections:
  +case OMPD_threadprivate:
  +case OMPD_allocate:
  +case OMPD_taskyield:
  +case OMPD_barrier:
  +case OMPD_taskwait:
  +case OMPD_cancellation_point:
  +case OMPD_flush:
  +case OMPD_declare_reduction:
  +case OMPD_declare_mapper:
  +case OMPD_declare_simd:
  +case OMPD_declare_variant:
  +case OMPD_declare_target:
  +case OMPD_end_declare_target:
  +case OMPD_sections:
  +case OMPD_section:
  +case OMPD_single:
  +case OMPD_master:
  +case OMPD_critical:
  +case OMPD_taskgroup:
  +case OMPD_distribute:
  +case OMPD_ordered:
  +case OMPD_atomic:
  +case OMPD_requires:
  +  llvm_unreachable("Unexpected OpenMP directive with linear-clause");
  +case OMPD_unknown:
  +  llvm_unreachable("Unknown OpenMP directive");
  +}
  +break;
 case OMPC_firstprivate:
 case OMPC_lastprivate:
 case OMPC_reduction:
 case OMPC_task_reduction:
 case OMPC_in_reduction:
  -  case OMPC_linear:
 case OMPC_default:
 case OMPC_proc_bind:
 case OMPC_safelen:
  @@ -14377,6 +14461,16 @@ OMPClause *Sema::ActOnOpenMPLinearClause(
   if (Val.isInvalid())
 return nullptr;
   StepExpr = Val.get();
  +OpenMPDirectiveKind DKind = DSAStack->getCurrentDirective();
  +OpenMPDirectiveKind CaptureRegion =
  +getOpenMPCaptureRegionForClause(DKind, OMPC_linear, LangOpts.OpenMP);
  +if (CaptureRegion != OMPD_unknown && !CurContext->isDependentContext()) {
  +  StepExpr = MakeFullExpr(StepExpr).get();
  +  llvm::MapVector Captures;
  +  StepExpr = tryBuildCapture(*this, StepExpr, Captures).get();
  +  for (const auto  : Captures)
  +ExprCaptures.push_back(Pair.second->getDecl());
  +}
  
   // Build var to save the step value.
   VarDecl *SaveVar =
  @@ -14503,7 +14597,7 @@ static bool FinishOpenMPLinearClause(OMPLinearClause 
, DeclRefExpr *IV,
   ++CurPrivate;
 }
 if (Expr *S = Clause.getStep())
  -UsedExprs.push_back(S);
  +UsedExprs.push_back(getExprAsWritten(S));
 // Fill the

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

2019-12-17 Thread Alexey Bataev via Phabricator via cfe-commits

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 && Data.Attributes == OMPC_firstprivate) ||

jdoerfert wrote:
> ABataev wrote:
> > jdoerfert wrote:
> > > ABataev wrote:
> > > > cchen wrote:
> > > > > ABataev wrote:
> > > > > > What is this change for?
> > > > > Since now the variable in step expression has been set as implicit 
> > > > > firstprivate.
> > > > No need to do this, the whole expression must be evaluated before 
> > > > entering the parallel region.
> > > The standard is not clear on this, e.g., what if the expression has a 
> > > side-effect and the loop has 0 iterations. However, evaluating it once in 
> > > the beginning seems fine to me, assuming we do not evaluate it later 
> > > again.
> > The standard says: `The linear-step expression must be invariant during the 
> > execution of the region that corresponds to the construct.` So, it is ok to 
> > evaluate it at the entrance to the region. The only thing that (maybe) 
> > required is to check that the expression is really invariant in the 
> > analysis phase.
> I'll just repeat the under-specified case you seem to have missed while 
> reading my comment:
> 
> > What if the expression has a side-effect and the loop has 0 iterations.
> 
> 
> 
> 
Yes, this is not specified


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71475/new/

https://reviews.llvm.org/D71475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-12-17 Thread Johannes Doerfert via Phabricator via cfe-commits

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 && Data.Attributes == OMPC_firstprivate) ||

ABataev wrote:
> jdoerfert wrote:
> > ABataev wrote:
> > > cchen wrote:
> > > > ABataev wrote:
> > > > > What is this change for?
> > > > Since now the variable in step expression has been set as implicit 
> > > > firstprivate.
> > > No need to do this, the whole expression must be evaluated before 
> > > entering the parallel region.
> > The standard is not clear on this, e.g., what if the expression has a 
> > side-effect and the loop has 0 iterations. However, evaluating it once in 
> > the beginning seems fine to me, assuming we do not evaluate it later again.
> The standard says: `The linear-step expression must be invariant during the 
> execution of the region that corresponds to the construct.` So, it is ok to 
> evaluate it at the entrance to the region. The only thing that (maybe) 
> required is to check that the expression is really invariant in the analysis 
> phase.
I'll just repeat the under-specified case you seem to have missed while reading 
my comment:

> What if the expression has a side-effect and the loop has 0 iterations.






Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71475/new/

https://reviews.llvm.org/D71475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-12-17 Thread Chi Chun Chen via Phabricator via cfe-commits

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 
> clauses caused by a non-perfect design. Add codegen tests for all combined 
> constructs that may require capturing of the linear step expression. 
> Probably, some additional work in codegen may be required.
>
>   diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
>   index afe0f1a..cb55ace 100644
>   --- a/clang/lib/Sema/SemaOpenMP.cpp
>   +++ b/clang/lib/Sema/SemaOpenMP.cpp
>   @@ -3830,6 +3830,9 @@ StmtResult Sema::ActOnOpenMPRegionEnd(StmtResult S,
>  MarkDeclarationsReferencedInExpr(E);
>}
>  }
>   +  if (auto *LC = dyn_cast(Clause))
>   +if (Expr *E = LC->getStep())
>   +  MarkDeclarationsReferencedInExpr(E);
>  DSAStack->setForceVarCapturing(/*V=*/false);
>} else if (CaptureRegions.size() > 1 ||
>   CaptureRegions.back() != OMPD_unknown) {
>   @@ -11396,12 +11399,87 @@ static OpenMPDirectiveKind 
> getOpenMPCaptureRegionForClause(
>  llvm_unreachable("Unknown OpenMP directive");
>}
>break;
>   +  case OMPC_linear:
>   +switch (DKind) {
>   +case OMPD_taskloop_simd:
>   +case OMPD_master_taskloop_simd:
>   +case OMPD_parallel_master_taskloop_simd:
>   +  CaptureRegion = OMPD_taskloop;
>   +  break;
>   +case OMPD_target_simd:
>   +  CaptureRegion = OMPD_target;
>   +  break;
>   +case OMPD_target_teams_distribute_simd:
>   +case OMPD_teams_distribute_simd:
>   +  CaptureRegion = OMPD_teams;
>   +  break;
>   +case OMPD_target_parallel_for:
>   +case OMPD_target_parallel_for_simd:
>   +case OMPD_target_teams_distribute_parallel_for:
>   +case OMPD_target_teams_distribute_parallel_for_simd:
>   +case OMPD_teams_distribute_parallel_for:
>   +case OMPD_teams_distribute_parallel_for_simd:
>   +case OMPD_distribute_parallel_for:
>   +case OMPD_distribute_parallel_for_simd:
>   +case OMPD_parallel_for:
>   +case OMPD_parallel_for_simd:
>   +  CaptureRegion = OMPD_parallel;
>   +  break;
>   +case OMPD_parallel_master_taskloop:
>   +case OMPD_task:
>   +case OMPD_taskloop:
>   +case OMPD_master_taskloop:
>   +case OMPD_target_update:
>   +case OMPD_target_enter_data:
>   +case OMPD_target_exit_data:
>   +case OMPD_target:
>   +case OMPD_target_teams:
>   +case OMPD_target_parallel:
>   +case OMPD_target_teams_distribute:
>   +case OMPD_target_data:
>   +case OMPD_teams:
>   +case OMPD_teams_distribute:
>   +case OMPD_cancel:
>   +case OMPD_parallel:
>   +case OMPD_parallel_master:
>   +case OMPD_parallel_sections:
>   +case OMPD_threadprivate:
>   +case OMPD_allocate:
>   +case OMPD_taskyield:
>   +case OMPD_barrier:
>   +case OMPD_taskwait:
>   +case OMPD_cancellation_point:
>   +case OMPD_flush:
>   +case OMPD_declare_reduction:
>   +case OMPD_declare_mapper:
>   +case OMPD_declare_simd:
>   +case OMPD_declare_variant:
>   +case OMPD_declare_target:
>   +case OMPD_end_declare_target:
>   +case OMPD_simd:
>   +case OMPD_for:
>   +case OMPD_for_simd:
>   +case OMPD_sections:
>   +case OMPD_section:
>   +case OMPD_single:
>   +case OMPD_master:
>   +case OMPD_critical:
>   +case OMPD_taskgroup:
>   +case OMPD_distribute:
>   +case OMPD_ordered:
>   +case OMPD_atomic:
>   +case OMPD_distribute_simd:
>   +case OMPD_requires:
>   +  llvm_unreachable("Unexpected OpenMP directive with linear-clause");
>   +case OMPD_unknown:
>   +  llvm_unreachable("Unknown OpenMP directive");
>   +}
>   +break;
>  case OMPC_firstprivate:
>  case OMPC_lastprivate:
>  case OMPC_reduction:
>  case OMPC_task_reduction:
>  case OMPC_in_reduction:
>   -  case OMPC_linear:
>  case OMPC_default:
>  case OMPC_proc_bind:
>  case OMPC_safelen:
>   @@ -14377,6 +14455,16 @@ OMPClause *Sema::ActOnOpenMPLinearClause(
>if (Val.isInvalid())
>  return nullptr;
>StepExpr = Val.get();
>   +OpenMPDirectiveKind DKind = DSAStack->getCurrentDirective();
>   +OpenMPDirectiveKind CaptureRegion =
>   +getOpenMPCaptureRegionForClause(DKind, OMPC_linear, 
> LangOpts.OpenMP);
>   +if (CaptureRegion != OMPD_unknown && 
> !CurContext->isDependentContext()) {
>   +  StepExpr = MakeFullExpr(StepExpr).get();
>   +  llvm::MapVector Captures;
>   +  StepExpr = tryBuildCapture(*this, StepExpr, Captures).get();
>   +  for (const auto  : Captures)
>   +ExprCaptures.push_back(Pair.second->getDecl());
>   +}
>  
>// Build var to save the

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

2019-12-17 Thread Alexey Bataev via Phabricator via cfe-commits

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 && Data.Attributes == OMPC_firstprivate) ||

jdoerfert wrote:
> ABataev wrote:
> > cchen wrote:
> > > ABataev wrote:
> > > > What is this change for?
> > > Since now the variable in step expression has been set as implicit 
> > > firstprivate.
> > No need to do this, the whole expression must be evaluated before entering 
> > the parallel region.
> The standard is not clear on this, e.g., what if the expression has a 
> side-effect and the loop has 0 iterations. However, evaluating it once in the 
> beginning seems fine to me, assuming we do not evaluate it later again.
The standard says: `The linear-step expression must be invariant during the 
execution of the region that corresponds to the construct.` So, it is ok to 
evaluate it at the entrance to the region. The only thing that (maybe) required 
is to check that the expression is really invariant in the analysis phase.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71475/new/

https://reviews.llvm.org/D71475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-12-17 Thread Johannes Doerfert via Phabricator via cfe-commits

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 && Data.Attributes == OMPC_firstprivate) ||

ABataev wrote:
> cchen wrote:
> > ABataev wrote:
> > > What is this change for?
> > Since now the variable in step expression has been set as implicit 
> > firstprivate.
> No need to do this, the whole expression must be evaluated before entering 
> the parallel region.
The standard is not clear on this, e.g., what if the expression has a 
side-effect and the loop has 0 iterations. However, evaluating it once in the 
beginning seems fine to me, assuming we do not evaluate it later again.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71475/new/

https://reviews.llvm.org/D71475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-12-17 Thread Alexey Bataev via Phabricator via cfe-commits

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 && Data.Attributes == OMPC_firstprivate) ||

cchen wrote:
> ABataev wrote:
> > What is this change for?
> Since now the variable in step expression has been set as implicit 
> firstprivate.
No need to do this, the whole expression must be evaluated before entering the 
parallel region.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71475/new/

https://reviews.llvm.org/D71475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-12-17 Thread Chi Chun Chen via Phabricator via cfe-commits

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)) ||
(A == OMPC_lastprivate && Data.Attributes == OMPC_firstprivate) ||

ABataev wrote:
> What is this change for?
Since now the variable in step expression has been set as implicit firstprivate.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71475/new/

https://reviews.llvm.org/D71475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-12-17 Thread Alexey Bataev via Phabricator via cfe-commits

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 
constructs that may require capturing of the linear step expression. Probably, 
some additional work in codegen may be required.

  diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
  index afe0f1a..cb55ace 100644
  --- a/clang/lib/Sema/SemaOpenMP.cpp
  +++ b/clang/lib/Sema/SemaOpenMP.cpp
  @@ -3830,6 +3830,9 @@ StmtResult Sema::ActOnOpenMPRegionEnd(StmtResult S,
 MarkDeclarationsReferencedInExpr(E);
   }
 }
  +  if (auto *LC = dyn_cast(Clause))
  +if (Expr *E = LC->getStep())
  +  MarkDeclarationsReferencedInExpr(E);
 DSAStack->setForceVarCapturing(/*V=*/false);
   } else if (CaptureRegions.size() > 1 ||
  CaptureRegions.back() != OMPD_unknown) {
  @@ -11396,12 +11399,87 @@ static OpenMPDirectiveKind 
getOpenMPCaptureRegionForClause(
 llvm_unreachable("Unknown OpenMP directive");
   }
   break;
  +  case OMPC_linear:
  +switch (DKind) {
  +case OMPD_taskloop_simd:
  +case OMPD_master_taskloop_simd:
  +case OMPD_parallel_master_taskloop_simd:
  +  CaptureRegion = OMPD_taskloop;
  +  break;
  +case OMPD_target_simd:
  +  CaptureRegion = OMPD_target;
  +  break;
  +case OMPD_target_teams_distribute_simd:
  +case OMPD_teams_distribute_simd:
  +  CaptureRegion = OMPD_teams;
  +  break;
  +case OMPD_target_parallel_for:
  +case OMPD_target_parallel_for_simd:
  +case OMPD_target_teams_distribute_parallel_for:
  +case OMPD_target_teams_distribute_parallel_for_simd:
  +case OMPD_teams_distribute_parallel_for:
  +case OMPD_teams_distribute_parallel_for_simd:
  +case OMPD_distribute_parallel_for:
  +case OMPD_distribute_parallel_for_simd:
  +case OMPD_parallel_for:
  +case OMPD_parallel_for_simd:
  +  CaptureRegion = OMPD_parallel;
  +  break;
  +case OMPD_parallel_master_taskloop:
  +case OMPD_task:
  +case OMPD_taskloop:
  +case OMPD_master_taskloop:
  +case OMPD_target_update:
  +case OMPD_target_enter_data:
  +case OMPD_target_exit_data:
  +case OMPD_target:
  +case OMPD_target_teams:
  +case OMPD_target_parallel:
  +case OMPD_target_teams_distribute:
  +case OMPD_target_data:
  +case OMPD_teams:
  +case OMPD_teams_distribute:
  +case OMPD_cancel:
  +case OMPD_parallel:
  +case OMPD_parallel_master:
  +case OMPD_parallel_sections:
  +case OMPD_threadprivate:
  +case OMPD_allocate:
  +case OMPD_taskyield:
  +case OMPD_barrier:
  +case OMPD_taskwait:
  +case OMPD_cancellation_point:
  +case OMPD_flush:
  +case OMPD_declare_reduction:
  +case OMPD_declare_mapper:
  +case OMPD_declare_simd:
  +case OMPD_declare_variant:
  +case OMPD_declare_target:
  +case OMPD_end_declare_target:
  +case OMPD_simd:
  +case OMPD_for:
  +case OMPD_for_simd:
  +case OMPD_sections:
  +case OMPD_section:
  +case OMPD_single:
  +case OMPD_master:
  +case OMPD_critical:
  +case OMPD_taskgroup:
  +case OMPD_distribute:
  +case OMPD_ordered:
  +case OMPD_atomic:
  +case OMPD_distribute_simd:
  +case OMPD_requires:
  +  llvm_unreachable("Unexpected OpenMP directive with linear-clause");
  +case OMPD_unknown:
  +  llvm_unreachable("Unknown OpenMP directive");
  +}
  +break;
 case OMPC_firstprivate:
 case OMPC_lastprivate:
 case OMPC_reduction:
 case OMPC_task_reduction:
 case OMPC_in_reduction:
  -  case OMPC_linear:
 case OMPC_default:
 case OMPC_proc_bind:
 case OMPC_safelen:
  @@ -14377,6 +14455,16 @@ OMPClause *Sema::ActOnOpenMPLinearClause(
   if (Val.isInvalid())
 return nullptr;
   StepExpr = Val.get();
  +OpenMPDirectiveKind DKind = DSAStack->getCurrentDirective();
  +OpenMPDirectiveKind CaptureRegion =
  +getOpenMPCaptureRegionForClause(DKind, OMPC_linear, LangOpts.OpenMP);
  +if (CaptureRegion != OMPD_unknown && !CurContext->isDependentContext()) {
  +  StepExpr = MakeFullExpr(StepExpr).get();
  +  llvm::MapVector Captures;
  +  StepExpr = tryBuildCapture(*this, StepExpr, Captures).get();
  +  for (const auto  : Captures)
  +ExprCaptures.push_back(Pair.second->getDecl());
  +}
  
   // Build var to save the step value.
   VarDecl *SaveVar =




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

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

2019-12-16 Thread Johannes Doerfert via Phabricator via cfe-commits

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 got the same result (return value) changing from firstprivate to 
> lastprivate. Not so sure how to make linear work with reduction.

I don't want it to "work" but I thought we should give a proper error message. 
Can you add test cases where a value is used in the step *and* in a (1) shared, 
(2) private, (3) firstprivate, ...?

Other than that I think the code changes look pretty good.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71475/new/

https://reviews.llvm.org/D71475

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-12-16 Thread Chi Chun Chen via Phabricator via cfe-commits

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:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/parallel_for_ast_print.cpp
  clang/test/OpenMP/parallel_for_linear_codegen.cpp
  clang/test/OpenMP/target_simd_ast_print.cpp

Index: clang/test/OpenMP/target_simd_ast_print.cpp
===
--- clang/test/OpenMP/target_simd_ast_print.cpp
+++ clang/test/OpenMP/target_simd_ast_print.cpp
@@ -187,6 +187,10 @@
   // CHECK-NEXT: for (T i = 0; i < N; ++i) {
   // CHECK-NEXT: }
 
+#pragma omp target simd linear (i: b + 1)
+  // CHECK: #pragma omp target simd linear(i: b + 1)
+  for (T j = 16; j < 64; j++)
+i += 4;
   return T();
 }
 
@@ -307,6 +311,10 @@
   // CHECK: #pragma omp target simd is_device_ptr(p)
   // CHECK-NEXT: for (int i = 0; i < 2; ++i) {
   // CHECK-NEXT: }
+#pragma omp target simd linear (i: b + 1)
+  // CHECK: #pragma omp target simd linear(i: b + 1)
+  for (int j = 16; j < 64; j++)
+i += 4;
 
   return (tmain(argc, ));
 }
Index: clang/test/OpenMP/parallel_for_linear_codegen.cpp
===
--- clang/test/OpenMP/parallel_for_linear_codegen.cpp
+++ clang/test/OpenMP/parallel_for_linear_codegen.cpp
@@ -28,6 +28,8 @@
 float f;
 char cnt;
 
+int a[100];
+
 // CHECK: [[S_FLOAT_TY:%.+]] = type { float }
 // CHECK: [[S_INT_TY:%.+]] = type { i32 }
 // CHECK-DAG: [[F:@.+]] = global float 0.0
@@ -255,3 +257,39 @@
 // CHECK: ret void
 #endif
 
+// RUN: %clang_cc1 -DCK1 -verify -fopenmp -x c++ -triple x86_64-apple-darwin10 -emit-llvm %s -o - | FileCheck %s -check-prefix=CK1
+// RUN: %clang_cc1 -DCK1 -fopenmp -x c++ -std=c++11 -triple x86_64-apple-darwin10 -emit-pch -o %t %s
+
+// RUN: %clang_cc1 -DCK1 -verify -fopenmp-simd -x c++ -triple x86_64-apple-darwin10 -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -DCK1 -fopenmp-simd -x c++ -std=c++11 -triple x86_64-apple-darwin10 -emit-pch -o %t %s
+// SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
+#ifdef CK1
+
+// CK1: foo
+int foo (int i, int k)
+{
+#pragma omp parallel for linear (i: k + 1)
+  for (int j = 16; j < 64; j++)
+  {
+a[i] = j;
+i += 4;
+  }
+  return i;
+}
+// CK1: define internal void [[OUTLINE:@.+]](i32* noalias [[GTID:%.+]], i32* noalias [[BTID:%.+]], i32* dereferenceable(4) [[I_VAR:%.+]], i32* dereferenceable(4) [[K_VAR:%.+]])
+// CK1: [[GTID_ADDR:%.+]] = alloca i32*
+// CK1: [[BTID_ADDR:%.+]] = alloca i32*
+// CK1: [[I_ADDR:%.+]] = alloca i32*
+// CK1: [[K_ADDR:%.+]] = alloca i32*
+// CK1: store i32* [[GTID]], i32** [[GTID_ADDR]]
+// CK1: store i32* [[BTID]], i32** [[BTID_ADDR]]
+// CK1: store i32* [[I_VAR:%.+]], i32** [[I_ADDR]]
+// CK1: store i32* [[K_VAR:%.+]], i32** [[K_ADDR]]
+// CK1: [[ZERO:%.+]] = load i32*, i32** [[I_ADDR]]
+// CK1: [[ONE:%.+]] = load i32*, i32** [[K_ADDR]]
+// CK1: [[TWO:%.+]] = load i32, i32* [[ZERO]]
+// CK1: store i32 [[TWO]], i32* [[LINEAR_START:%.+]]
+// CK1: [[THREE:%.+]] = load i32, i32* [[ONE]]
+// CK1: [[ADD:%.+]] = add nsw i32 [[THREE]]
+// CK1: store i32 [[ADD]], i32* [[LINEAR_STEP:%.+]]
+#endif
Index: clang/test/OpenMP/parallel_for_ast_print.cpp
===
--- clang/test/OpenMP/parallel_for_ast_print.cpp
+++ clang/test/OpenMP/parallel_for_ast_print.cpp
@@ -100,6 +100,11 @@
   // CHECK-NEXT: for (int j = 0; j < 2; ++j)
   // CHECK-NEXT: for (int j = 0; j < 2; ++j)
   // CHECK-NEXT: foo();
+
+  T i;
+#pragma omp parallel for linear (i: b + 1)
+  for (T j = 16; j < 64; j++)
+b += 4;
   return T();
 }
 
@@ -146,6 +151,11 @@
  // CHECK-NEXT: for (int i = 0; i < 10; ++i)
   // CHECK-NEXT: for (int j = 0; j < 10; ++j)
   // CHECK-NEXT: foo();
+
+  int i;
+#pragma omp parallel for linear (i: b + 1)
+  for (int j = 16; j < 64; j++)
+b += 4;
   return (tmain(argc) + tmain(argv[0][0]));
 }
 
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -1124,7 +1124,8 @@
   } else {
 DSAInfo  = getTopOfStack().SharingMap[D];
 assert(Data.Attributes == OMPC_unknown || (A == Data.Attributes) ||
-   (A == OMPC_firstprivate && Data.Attributes == OMPC_lastprivate) ||
+   (A == OMPC_firstprivate && (Data.Attributes == OMPC_lastprivate ||
+   Data.Attributes == OMPC_linear)) ||
(A == OMPC_lastprivate && Data.Attributes == OMPC_firstprivate) ||
(isLoopControlVariable(D).first && A == OMPC_private));
 if (A == OMPC_lastprivate && Data.Attributes == OMPC_firstprivate) {
@@ -3829,6 +3830,9 @@
   MarkDeclarationsReferencedInExpr(E);
 }
   }
+  if (auto *LC =

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

2019-12-16 Thread Chi Chun Chen via Phabricator via cfe-commits

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
  https://reviews.llvm.org/D71475/new/

https://reviews.llvm.org/D71475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-12-13 Thread Alexey Bataev via Phabricator via cfe-commits

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 Sema::ActOnOpenMPRegionEnd(StmtResult S,
 MarkDeclarationsReferencedInExpr(E);
   }
 }
  +  if (auto *LC = dyn_cast(Clause))
  +if (Expr *E = LC->getStep())
  +  MarkDeclarationsReferencedInExpr(E);
 DSAStack->setForceVarCapturing(/*V=*/false);
   } else if (CaptureRegions.size() > 1 ||
  CaptureRegions.back() != OMPD_unknown) {


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71475/new/

https://reviews.llvm.org/D71475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-12-13 Thread Chi Chun Chen via Phabricator via cfe-commits

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 firstprivate to 
lastprivate. Not so sure how to make linear work with reduction.

> In D71475#1784173 , @cchen wrote:
> 
>> Doing this still fail the assertion since we still don't have the variable 
>> inside
>>  CapturedStmt.
> 
> 
> So we need to mark it as captured as well.

I've tried to use the `buildCapture` function before, but seems not work for 
me, can you point out where to look at in the source code? Thanks




Comment at: clang/lib/Sema/SemaOpenMP.cpp:4512
   }
 }
 int ClauseKindCnt = -1;

jdoerfert wrote:
> Is this "ErrorFound" here set when you run the example?
ErrorFound does not set. The firstprivate node is correctly set by checking 
ast-dump


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71475/new/

https://reviews.llvm.org/D71475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-12-13 Thread Johannes Doerfert via Phabricator via cfe-commits

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.


So we need to mark it as captured as well.




Comment at: clang/lib/Sema/SemaOpenMP.cpp:4512
   }
 }
 int ClauseKindCnt = -1;

Is this "ErrorFound" here set when you run the example?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71475/new/

https://reviews.llvm.org/D71475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-12-13 Thread Chi Chun Chen via Phabricator via cfe-commits

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
  clang/test/OpenMP/parallel_for_linear_codegen.cpp


Index: clang/test/OpenMP/parallel_for_linear_codegen.cpp
===
--- clang/test/OpenMP/parallel_for_linear_codegen.cpp
+++ clang/test/OpenMP/parallel_for_linear_codegen.cpp
@@ -28,6 +28,19 @@
 float f;
 char cnt;
 
+int a[100];
+
+int foo (int i, int k)
+{
+#pragma omp parallel for linear (i: k + 1)
+  for (int j = 16; j < 64; j++)
+  {
+a[i] = j;
+i += 4;
+  }
+  return i;
+}
+
 // CHECK: [[S_FLOAT_TY:%.+]] = type { float }
 // CHECK: [[S_INT_TY:%.+]] = type { i32 }
 // CHECK-DAG: [[F:@.+]] = global float 0.0
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -4404,6 +4404,32 @@
   }
 }
 
+namespace {
+class LinearStepVarChecker : public StmtVisitor {
+  llvm::SmallVector ImplicitFirstprivate;
+public:
+  bool VisitDeclRefExpr(DeclRefExpr *E) {
+if (auto *VD = dyn_cast(E->getDecl())) {
+  ImplicitFirstprivate.push_back(cast(E));
+  return true;
+}
+return false;
+  }
+  bool VisitStmt(Stmt *S) {
+for (Stmt *Child : S->children()) {
+  if (Child && Visit(Child))
+return true;
+}
+return false;
+  }
+  ArrayRef getImplicitFirstprivate() const {
+return ImplicitFirstprivate;
+  }
+
+  explicit LinearStepVarChecker() {}
+};
+} // namespace
+
 StmtResult Sema::ActOnOpenMPExecutableDirective(
 OpenMPDirectiveKind Kind, const DeclarationNameInfo ,
 OpenMPDirectiveKind CancelRegion, ArrayRef Clauses,
@@ -4460,6 +4486,17 @@
 for (Expr *E : IRC->taskgroup_descriptors())
   if (E)
 ImplicitFirstprivates.emplace_back(E);
+  } else if (auto *LC = dyn_cast(C)) {
+Expr *E = LC->getStep();
+if (E) {
+  LinearStepVarChecker LSVChecker;
+  LSVChecker.Visit(E);
+  ArrayRef LinearVars = LSVChecker.getImplicitFirstprivate();
+  ImplicitFirstprivates.insert(
+  ImplicitFirstprivates.end(),
+  std::make_move_iterator(LinearVars.begin()),
+  std::make_move_iterator(LinearVars.end()));
+}
   }
 }
 if (!ImplicitFirstprivates.empty()) {


Index: clang/test/OpenMP/parallel_for_linear_codegen.cpp
===
--- clang/test/OpenMP/parallel_for_linear_codegen.cpp
+++ clang/test/OpenMP/parallel_for_linear_codegen.cpp
@@ -28,6 +28,19 @@
 float f;
 char cnt;
 
+int a[100];
+
+int foo (int i, int k)
+{
+#pragma omp parallel for linear (i: k + 1)
+  for (int j = 16; j < 64; j++)
+  {
+a[i] = j;
+i += 4;
+  }
+  return i;
+}
+
 // CHECK: [[S_FLOAT_TY:%.+]] = type { float }
 // CHECK: [[S_INT_TY:%.+]] = type { i32 }
 // CHECK-DAG: [[F:@.+]] = global float 0.0
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -4404,6 +4404,32 @@
   }
 }
 
+namespace {
+class LinearStepVarChecker : public StmtVisitor {
+  llvm::SmallVector ImplicitFirstprivate;
+public:
+  bool VisitDeclRefExpr(DeclRefExpr *E) {
+if (auto *VD = dyn_cast(E->getDecl())) {
+  ImplicitFirstprivate.push_back(cast(E));
+  return true;
+}
+return false;
+  }
+  bool VisitStmt(Stmt *S) {
+for (Stmt *Child : S->children()) {
+  if (Child && Visit(Child))
+return true;
+}
+return false;
+  }
+  ArrayRef getImplicitFirstprivate() const {
+return ImplicitFirstprivate;
+  }
+
+  explicit LinearStepVarChecker() {}
+};
+} // namespace
+
 StmtResult Sema::ActOnOpenMPExecutableDirective(
 OpenMPDirectiveKind Kind, const DeclarationNameInfo ,
 OpenMPDirectiveKind CancelRegion, ArrayRef Clauses,
@@ -4460,6 +4486,17 @@
 for (Expr *E : IRC->taskgroup_descriptors())
   if (E)
 ImplicitFirstprivates.emplace_back(E);
+  } else if (auto *LC = dyn_cast(C)) {
+Expr *E = LC->getStep();
+if (E) {
+  LinearStepVarChecker LSVChecker;
+  LSVChecker.Visit(E);
+  ArrayRef LinearVars = LSVChecker.getImplicitFirstprivate();
+  ImplicitFirstprivates.insert(
+  ImplicitFirstprivates.end(),
+  std::make_move_iterator(LinearVars.begin()),
+  std::make_move_iterator(LinearVars.end()));
+}
   }
 }
 if (!ImplicitFirstprivates.empty()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-12-13 Thread Chi Chun Chen via Phabricator via cfe-commits

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
  https://reviews.llvm.org/D71475/new/

https://reviews.llvm.org/D71475

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/parallel_for_linear_codegen.cpp


Index: clang/test/OpenMP/parallel_for_linear_codegen.cpp
===
--- clang/test/OpenMP/parallel_for_linear_codegen.cpp
+++ clang/test/OpenMP/parallel_for_linear_codegen.cpp
@@ -28,6 +28,19 @@
 float f;
 char cnt;
 
+int a[100];
+
+int foo (int i, int k)
+{
+#pragma omp parallel for linear (i: k + 1)
+  for (int j = 16; j < 64; j++)
+  {
+a[i] = j;
+i += 4;
+  }
+  return i;
+}
+
 // CHECK: [[S_FLOAT_TY:%.+]] = type { float }
 // CHECK: [[S_INT_TY:%.+]] = type { i32 }
 // CHECK-DAG: [[F:@.+]] = global float 0.0
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -4404,6 +4404,34 @@
   }
 }
 
+namespace {
+class LinearStepVarChecker : public StmtVisitor {
+  llvm::SmallVector ImplicitFirstprivate;
+public:
+  bool VisitDeclRefExpr(DeclRefExpr *E) {
+if (auto *VD = dyn_cast(E->getDecl())) {
+  llvm::errs() << "VisitDeclRefExpr: \n";
+  VD->dump();
+  ImplicitFirstprivate.push_back(cast(E));
+  return true;
+}
+return false;
+  }
+  bool VisitStmt(Stmt *S) {
+for (Stmt *Child : S->children()) {
+  if (Child && Visit(Child))
+return true;
+}
+return false;
+  }
+  ArrayRef getImplicitFirstprivate() const {
+return ImplicitFirstprivate;
+  }
+
+  explicit LinearStepVarChecker() {}
+};
+} // namespace
+
 StmtResult Sema::ActOnOpenMPExecutableDirective(
 OpenMPDirectiveKind Kind, const DeclarationNameInfo ,
 OpenMPDirectiveKind CancelRegion, ArrayRef Clauses,
@@ -4460,6 +4488,17 @@
 for (Expr *E : IRC->taskgroup_descriptors())
   if (E)
 ImplicitFirstprivates.emplace_back(E);
+  } else if (auto *LC = dyn_cast(C)) {
+Expr *E = LC->getStep();
+if (E) {
+  LinearStepVarChecker LSVChecker;
+  LSVChecker.Visit(E);
+  ArrayRef LinearVars = LSVChecker.getImplicitFirstprivate();
+  ImplicitFirstprivates.insert(
+  ImplicitFirstprivates.end(),
+  std::make_move_iterator(LinearVars.begin()),
+  std::make_move_iterator(LinearVars.end()));
+}
   }
 }
 if (!ImplicitFirstprivates.empty()) {
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtOpenMP.h"
 #include "clang/Basic/PrettyStackTrace.h"
+#include 
 using namespace clang;
 using namespace CodeGen;
 using namespace llvm::omp;


Index: clang/test/OpenMP/parallel_for_linear_codegen.cpp
===
--- clang/test/OpenMP/parallel_for_linear_codegen.cpp
+++ clang/test/OpenMP/parallel_for_linear_codegen.cpp
@@ -28,6 +28,19 @@
 float f;
 char cnt;
 
+int a[100];
+
+int foo (int i, int k)
+{
+#pragma omp parallel for linear (i: k + 1)
+  for (int j = 16; j < 64; j++)
+  {
+a[i] = j;
+i += 4;
+  }
+  return i;
+}
+
 // CHECK: [[S_FLOAT_TY:%.+]] = type { float }
 // CHECK: [[S_INT_TY:%.+]] = type { i32 }
 // CHECK-DAG: [[F:@.+]] = global float 0.0
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -4404,6 +4404,34 @@
   }
 }
 
+namespace {
+class LinearStepVarChecker : public StmtVisitor {
+  llvm::SmallVector ImplicitFirstprivate;
+public:
+  bool VisitDeclRefExpr(DeclRefExpr *E) {
+if (auto *VD = dyn_cast(E->getDecl())) {
+  llvm::errs() << "VisitDeclRefExpr: \n";
+  VD->dump();
+  ImplicitFirstprivate.push_back(cast(E));
+  return true;
+}
+return false;
+  }
+  bool VisitStmt(Stmt *S) {
+for (Stmt *Child : S->children()) {
+  if (Child && Visit(Child))
+return true;
+}
+return false;
+  }
+  ArrayRef getImplicitFirstprivate() const {
+return ImplicitFirstprivate;
+  }
+
+  explicit LinearStepVarChecker() {}
+};
+} // namespace
+
 StmtResult Sema::ActOnOpenMPExecutableDirective(
 OpenMPDirectiveKind Kind, const DeclarationNameInfo ,
 OpenMPDirectiveKind CancelRegion, ArrayRef Clauses,
@@ -4460,6 +4488,17 @@
 for (Expr *E : IRC->taskgroup_descriptors())
   if (E)
 ImplicitFirstprivates.emplace_back(E);
+  } else if (auto *LC = dyn_cast(C))

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

2019-12-13 Thread Johannes Doerfert via Phabricator via cfe-commits

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 firstprivate? (I mean, you can 
> > try it in C first).
> >
> > And how do I reproduce the crash? I tried: https://godbolt.org/z/FDPSnA
>
>
> Add firstprivate make the code work. Also, the code crash due to assertion 
> failure and I guess the compiler explorer is using release version instead of 
> debug version?

I see. That makes sense.

What do you think about making all variables used in the linear-step 
(implicitly) firstprivate? Doing this might allow us to (easily) verify they 
are not shared/private/lastprivate etc. already. If I run this with k 
lastprivate there is no assertion but the code is not updating k, potentially 
because it is not legal but then we want to error out. k in a reduction is 
similar.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71475/new/

https://reviews.llvm.org/D71475

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-12-13 Thread Chi Chun Chen via Phabricator via cfe-commits

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 tried: https://godbolt.org/z/FDPSnA


Add firstprivate make the code work. Also, the code crash due to assertion 
failure and I guess the compiler explorer is using release version instead of 
debug version?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71475/new/

https://reviews.llvm.org/D71475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-12-13 Thread Johannes Doerfert via Phabricator via cfe-commits

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 LAST ACTION
  https://reviews.llvm.org/D71475/new/

https://reviews.llvm.org/D71475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-12-13 Thread Chi Chun Chen via Phabricator via cfe-commits

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++)
  {
  a[i] = j;
  i += 4;
  }
  return i;
  }

Clang will crash since it does not capture k in OpenMP outlined
function (failed assertion: "DeclRefExpr for Decl not entered in 
LocalDeclMap?").
By evaluating k inside the for loop, the code can run without issue.
Therefore, my change in CGStmtOpenMP.cpp is just inserting a alloca for
k to make sure the issue is due to not capturing the variable correctly.

I think the correct way might be adding a checker in SemaOpenMP to find if
there is any step expression contain any non-constant var and add them to
the parameter of OpenMP outlined function. However, I haven't figured
out how to add the var as parameter of OpenMP outlined function 
(ActOnOpenMPRegionStart
is for directive not for clause).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71475

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/parallel_for_linear_codegen.cpp


Index: clang/test/OpenMP/parallel_for_linear_codegen.cpp
===
--- clang/test/OpenMP/parallel_for_linear_codegen.cpp
+++ clang/test/OpenMP/parallel_for_linear_codegen.cpp
@@ -28,6 +28,19 @@
 float f;
 char cnt;
 
+int a[100];
+
+int foo (int i, int k)
+{
+#pragma omp parallel for linear (i: k + 1)
+  for (int j = 16; j < 64; j++)
+  {
+a[i] = j;
+i += 4;
+  }
+  return i;
+}
+
 // CHECK: [[S_FLOAT_TY:%.+]] = type { float }
 // CHECK: [[S_INT_TY:%.+]] = type { i32 }
 // CHECK-DAG: [[F:@.+]] = global float 0.0
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtOpenMP.h"
 #include "clang/Basic/PrettyStackTrace.h"
+#include 
 using namespace clang;
 using namespace CodeGen;
 using namespace llvm::omp;
@@ -1501,6 +1502,39 @@
 if (const auto *CS = cast_or_null(C->getCalcStep()))
   if (const auto *SaveRef = cast(CS->getLHS())) {
 EmitVarDecl(*cast(SaveRef->getDecl()));
+// Run DFS to find all the non-constant node under linear-step 
expression
+const Expr* Step = CS->getRHS();
+std::queue StmtQueue;
+StmtQueue.push(Step);
+while (!StmtQueue.empty()) {
+  const Stmt* CurStep = StmtQueue.front();
+  StmtQueue.pop();
+  if (const auto *BO = dyn_cast(CurStep)) {
+StmtQueue.push(BO->getLHS());
+StmtQueue.push(BO->getRHS());
+  } else if (isa(CurStep)) {
+const auto *IC = dyn_cast(CurStep);
+for (const Stmt *Child: IC->children()) {
+  StmtQueue.push(Child);
+}
+  } else if (isa(CurStep)) {
+// Generate a `alloca` for the variable in linear-step that
+// is not inside LocalDeclMap since compiler does not capture
+// this variable inside openmp-construct AST (which also
+// don't consider this variable as LValue either).
+// TODO The runtime behavior is incorrect for now since I only add
+// an `alloca`, and haven't store a meaningful value yet.
+if (const auto *DRE = dyn_cast(CurStep)) {
+  const ValueDecl* VD = DRE->getDecl();
+  const VarDecl* VarD = cast(VD);
+  if (LocalDeclMap.find(VarD) == LocalDeclMap.end()) {
+AutoVarEmission Emission = EmitAutoVarAlloca(*VarD);
+LocalDeclMap.insert({VarD, Emission.getAllocatedAddress()});
+EmitAutoVarCleanups(Emission);
+  }
+}
+  }
+}
 // Emit calculation of the linear step.
 EmitIgnoredExpr(CS);
   }


Index: clang/test/OpenMP/parallel_for_linear_codegen.cpp
===
--- clang/test/OpenMP/parallel_for_linear_codegen.cpp
+++ clang/test/OpenMP/parallel_for_linear_codegen.cpp
@@ -28,6 +28,19 @@
 float f;
 char cnt;
 
+int a[100];
+
+int foo (int i, int k)
+{
+#pragma omp parallel for linear (i: k + 1)
+  for (int j = 16; j < 64; j++)
+  {
+a[i] = j;
+i += 4;
+  }
+  return i;
+}
+
 // CHECK: [[S_FLOAT_TY:%.+]] = type { float }
 // CHECK: [[S_INT_TY:%.+]] = type { i32 }
 // CHECK-DAG: [[F:@.+]] = global float 0.0
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtOpenMP.h"
 #include "clang/Basic/PrettyStackTrace.h"
+#include 
 using

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

23 matches

Site Navigation

Mail list logo

Footer information