[PATCH] D145264: [OpenMP][MLIR][Flang][Driver][bbc] Lower and apply Module FlagsAttr

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon added inline comments.


================
Comment at: clang/include/clang/Driver/Options.td:2692
 def fno_openmp_assume_teams_oversubscription : Flag<["-"], "fno-openmp-assume-teams-oversubscription">,
-  Group<f_Group>, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
+  Group<f_Group>, Flags<[CC1Option, FC1Option, NoArgumentUnused, HelpHidden]>;
 def fno_openmp_assume_threads_oversubscription : Flag<["-"], "fno-openmp-assume-threads-oversubscription">,
----------------
awarzynski wrote:
> Many of these options have identical flags. While not really needed for this change, it would still be nice to re-organise them a bit. This file could really benefit from some love :) Here's an example of what I have in mind: https://github.com/llvm/llvm-project/blob/cf60d3f1a688671c8eb7859bf0572c403c3c0cca/clang/include/clang/Driver/Options.td#L6575-L6600
I can have a look into doing this in this patch :-)


================
Comment at: flang/tools/bbc/CMakeLists.txt:29
 FortranLower
+flangFrontendTool
 )
----------------
awarzynski wrote:
> This a frontend driver library and so far `bbc` and `flang-new -fc1` have been entirely separate. Could this dependency be avoided?
I had hoped to share LangOpts so that the setOffloadModuleInterfaceAttributes function wouldn't turn into a monolithic set of arguments whenever it's invoked if more arguments are added, but the dependency isn't ideal I agree!

I could perhaps look into making some sort of shared data structure to be put inside of CrossToolHelpers that might remove the dependency and be similarly useable to how LangOpts works at the moment. If that doesn't work, I can revert the change to just be a regular argument list and we can revisit the topic if new options are ever added? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145264