jdenny marked 4 inline comments as done. jdenny added inline comments.
================ Comment at: clang/lib/Basic/OpenMPKinds.cpp:64 + return OMPC_MAP_MODIFIER_unknown; + if (!LangOpts.OpenMPExtensions && Type == OMPC_MAP_MODIFIER_ompx_hold) return OMPC_MAP_MODIFIER_unknown; ---------------- ABataev wrote: > jdenny wrote: > > ABataev wrote: > > > jdenny wrote: > > > > ABataev wrote: > > > > > I would enable this since OpenMP 5.2, since in 5.2 `ompx_` is > > > > > officially allowed extension format. > > > > Do you mean both `-fopenmp-version=52` and `-fopenmp-extensions` should > > > > be required to enable `ompx_hold`? > > > > > > > > Or do you mean only `-fopenmp-version=52` should be required to enable > > > > `ompx_hold`, and we can drop the `-fopenmp-extensions` implementation? > > > Second option. Actually, we can enable it if either `-fopenmp-version=52` > > > or `-fopenmp-extensions` is used, depends if we want to add a switch for > > > non-standard OpenMP extensions. If OpenMP 5.2 is on, we can just ignore > > > `-f[no]-openmp-extensions`. Thoughts? > > Consider that, if `-fopenmp-version=52` is sufficient to enable all > > extensions using the `ompx_` prefix, then all such extensions will be > > enabled by default once OpenMP 5.2 is the default. At that point, won't it > > be strange that standard OpenMP 5.3 or 6.0 features will be disabled by > > default while some features appearing in no standard will be enabled by > > default? > > > > By that logic, it seems `-fopenmp-version=52` shouldn't be sufficient to > > enable extensions. However, it seems overkill to have to specify both > > `-fopenmp-version=52` and `-fopenmp-extensions`. I think > > `-fopenmp-extensions` is a clear enough request to enable the `ompx_` > > prefix regardless of the OpenMP version. > Ok, let's move with `-fopenmp-extensions` OK, thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106509/new/ https://reviews.llvm.org/D106509 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits