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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits