[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

ABataev added a comment. In D66559#1665289 , @cchen wrote: > Oh, I was thinking that I could use "arc land" to commit my patch, but just > now realize that I don't have the commit privileges. Would you please commit > for me? Thanks. Sure, will do this

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

cchen added a comment. Oh, I was thinking that I could use "arc land" to commit my patch, but just now realize that I don't have the commit privileges. Would you please commit for me? Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66559/new

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

ABataev added a comment. In D66559#1663887 , @cchen wrote: > @ABataev, could you mark it as ready to land, please. Thanks. I accepted it already, don't know what else I can do. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

cchen marked an inline comment as done. cchen added a comment. @ABataev, could you mark it as ready to land, please. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66559/new/ https://reviews.llvm.org/D66559 ___

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

ABataev accepted this revision. ABataev added a comment. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66559/new/ https://reviews.llvm.org/D66559 ___ cfe-commits mailing list cfe-commits@lists.llvm.

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:5423 + SemaRef.Diag(DefaultLoc, diag::err_omp_loop_not_canonical_cond) + << IneqCondIsCanonical << LCDecl; return true; I would not suggest to rely o

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

cchen marked an inline comment as done. cchen added a comment. I'm not able to `arc land` my patch since I updated the patch for a minor refactor and the state is "not approve" now. Do I wait for the approval for the new patch so that I can `arc land` or I don't need to do `arc land` myself? Tha

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. LG, thanks for the fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66559/new/ https://reviews.llvm.org/D66559 __

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

ABataev added a comment. In D66559#1651756 , @cchen wrote: > "parallel_for_codegen" cannot pass now since it has two test cases for "!=" > and it's hard to just add "fopenmp-version=50" since it will break lots of > other test cases that have different c

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

lebedev.ri resigned from this revision. lebedev.ri added a comment. Looks about right now, thanks for unbreaking it. Comment at: clang/lib/Sema/SemaOpenMP.cpp:5419-5420 // b relational-op var // if (!S) { + SemaRef.Diag(DefaultLoc, diag::err_omp_loop_no

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

cchen added a comment. "parallel_for_codegen" cannot pass now since it has two test cases for "!=" and it's hard to just add "fopenmp-version=50" since it will break lots of other test cases that have different codegen for OpenMP 5.0. I can remove the "!=" case from "parallel_for_codegen" and ad

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

cchen added a comment. @ABataev thanks so much for your instruction, I'll look into that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66559/new/ https://reviews.llvm.org/D66559 ___ cfe-commits mailin

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

ABataev added a comment. In D66559#1649775 , @cchen wrote: > Haven't updated the lit test for OpenMP 5.0 yet since I'm not able to check > OPENMP version by using _OPENMP with preprocessor condition checking. You don't need `_OPENMP` macro for this. The

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

ABataev added a comment. What about the check for `!=`? it would be good to allow it only for OpenMP 5.0. Also, add/update the tests. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9027-9032 +def err_omp_before_50_loop_not_canonical_cond : Error< "condition of

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

cchen added a comment. Haven't updated the lit test for OpenMP 5.0 yet since I'm not able to check OPENMP version by using _OPENMP with preprocessor condition checking. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66559/new/ https://reviews.llvm.

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

lebedev.ri added a comment. `!=` is still not being diagnosed in pre-5.0 mode though? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66559/new/ https://reviews.llvm.org/D66559 ___ cfe-commits mailing li

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

cchen updated this revision to Diff 217710. cchen added a comment. Update the diagnosis message for canonical loop form based on OpenMP version Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66559/new/ https://reviews.llvm.org/D66559 Files: clang

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

ABataev added a comment. In D66559#1641827 , @cchen wrote: > Is there any way that I can find the OpenMP version in Clang? I couldn't find > any example in source code. Thanks. LangOpts.OpenMP has the version of OpenMP. For OpenMP 5.0 this value is set

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

cchen added a comment. Is there any way that I can find the OpenMP version in Clang? I couldn't find any example in source code. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66559/new/ https://reviews.llvm.org/D66559 __

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

ABataev added a comment. `!=` form is part of OpenMP 5.0. It would be good to allow this form for OpenMP 5.0 only and emit this new error message only for OpenMP 5.0 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66559/new/ https://reviews.llvm.org

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

ABataev added a comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66559/new/ https://reviews.llvm.org/D66559 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/ma

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

lebedev.ri added a comment. Or more specifically, why `!=` is now silently accepted in 4.0 mode? https://godbolt.org/z/M6iL4H Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66559/new/ https://reviews.llvm.org/D66559 __

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

lebedev.ri added a comment. Shouldn't this be OpenMP-version-dependent? `!=` is only allowed in 4.5+ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66559/new/ https://reviews.llvm.org/D66559 ___ cfe-com

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

cchen created this revision. Herald added subscribers: cfe-commits, guansong. Herald added a reviewer: jdoerfert. Herald added a project: clang. The previous patch (https://reviews.llvm.org/D54441) support the relational-op != very well for openmp canonical loop form, however, it didn't update the