[PATCH] D61522: Added an assertion to constant evaluation enty points that prohibits dependent expressions
Dmitri Gribenko via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed May 8 02:23:40 PDT 2019
gribozavr marked an inline comment as done.
gribozavr added a comment.
In D61522#1494155 <https://reviews.llvm.org/D61522#1494155>, @rsmith wrote:
> The right thing to check in all of these cases should be only `isValueDependent()`. Every type-dependent expression should generally also be value-dependent (because the type is part of the value), but value-dependent exactly means "dependent in a way that prevents constant evaluation", so that's all that we should be checking.
Done. PTAL.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:5784
Expr::EvalResult Result;
- if (CollapseLoopCountExpr->EvaluateAsInt(Result, SemaRef.getASTContext()))
+ if (!CollapseLoopCountExpr->isValueDependent() &&
+ !CollapseLoopCountExpr->isTypeDependent() &&
----------------
ABataev wrote:
> gribozavr wrote:
> > ABataev wrote:
> > > I would suggest to modify the code of this function if we cannot get the value of the loops.
> > > ```
> > > if (CollapseLoopCountExpr->isValueDependent() || CollapseLoopCountExpr->isTypeDependent() || OrderedLoopCountExpr->isValueDependent() || OrderedLoopCountExpr->isTypeDependent()) {
> > > Built.clear(/* size */0);
> > > return 1;
> > > }
> > > ```
> > > at the beginning of the function.
> > I tried doing that, and a lot of tests started crashing with:
> >
> > llvm-project/clang/lib/Sema/SemaOpenMP.cpp:9024: clang::StmtResult clang::Sema::ActOnOpenMPTargetTeamsDistributeSimdDirective(ArrayRef<clang::OMPClause *>, clang::Stmt *, clang::SourceLoc
> > ation, clang::SourceLocation, clang::Sema::VarsWithInheritedDSAType &): Assertion `(CurContext->isDependentContext() || B.builtAll()) && "omp target teams distribute simd loop exprs were not built"' failed.
> >
> > Also, I wanted to note that if I were to make this change, then if `EvaluateAsInt` fails, we should apply the same recovery (`Built.clear(); return`).
> The just try to use `Built.clear(/* size */1);`, I assume it must fix the problems.
Yes, this works! Thanks!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61522/new/
https://reviews.llvm.org/D61522
More information about the cfe-commits
mailing list