[PATCH] D106509: [OpenMP][OpenACC] Implement `ompx_hold` map type modifier extension in Clang (1/2)

[Joel E. Denny via Phabricator via cfe-commits]

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:
> > > 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.


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

https://reviews.llvm.org/D106509