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

Reply via email to