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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D96588: [OpenCL... Sven van Haastregt via Phabricator via cfe-commits
    • [PATCH] D96588: [O... Anastasia Stulova via Phabricator via cfe-commits

Reply via email to