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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to