[PATCH] D96588: [OpenCL] Remove FIXME in getOpenCLFeatureDefines

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

> The suggestion in D92277 <https://reviews.llvm.org/D92277> was to move OpenCLOptions into LanguageOptions, but



> this is not viable. Sema's LangOpts is immutable, and embedding
> OpenCLOptions into LangOpts would make OpenCLOptions immutable too.
> This is incompatible with handling of pragmas that alter the
> OpenCLOptions during parsing/sema.

That's right in general `OpenCLOptions` should become a part of `LangOptions` as they are the same conceptually. `CompilerInvocation` has `LangOptions` as non-const member and it sets many of its fields. So it could do the same for OpenCL specific options as it also contains `TargetOptions` with `OpenCLFeaturesMap` so it know what the targets support and, therefore, has the full information needed to complete the right setup.

I agree dynamic modifications of `OpenCLOptions` via pragmas makes things more complicated. However, we could do something similar to  `CurFPFeatures` in `Sema` that duplicates some of floating point options of the `LangOptions` and provides the functionality of dynamic modification via pragmas too. The OpenCL use case is exactly the same.  In reality, very few options would need the pragma so the number of items in that extra data structure would be very small. But unfortunately, this is not where we are right now yet. Because with time a lot of pragma uses have been added without good justification. So perhaps it is not a practical refactoring at the moment. But maybe it is something we could still do in the future if we modify the pragma use cases?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96588