[PATCH] D123211: [flang][driver] Add support for generating LLVM bytecode files

ekieri added inline comments. Comment at: flang/lib/Frontend/FrontendActions.cpp:491 + // Create and configure `TargetMachine` + std::unique_ptr TM; + awarzynski wrote: > ekieri wrote: > > awarzynski wrote: > > > ekieri wrote: > > > > Is there a reason why use

[PATCH] D123211: [flang][driver] Add support for generating LLVM bytecode files

This revision was automatically updated to reflect the committed changes. Closed by commit rGdd56939a4b04: [flang][driver] Add support for generating LLVM bytecode files (authored by awarzynski). Changed prior to commit: https://reviews.llvm.org/D123211?vs=420862&id=422453#toc Repository: rG

[PATCH] D123211: [flang][driver] Add support for generating LLVM bytecode files

awarzynski added inline comments. Comment at: flang/lib/Frontend/FrontendActions.cpp:491 + // Create and configure `TargetMachine` + std::unique_ptr TM; + ekieri wrote: > awarzynski wrote: > > ekieri wrote: > > > Is there a reason why use TM.reset instead of in

[PATCH] D123211: [flang][driver] Add support for generating LLVM bytecode files

ekieri added inline comments. Comment at: flang/lib/Frontend/FrontendActions.cpp:491 + // Create and configure `TargetMachine` + std::unique_ptr TM; + awarzynski wrote: > ekieri wrote: > > Is there a reason why use TM.reset instead of initialising TM like you d

[PATCH] D123211: [flang][driver] Add support for generating LLVM bytecode files

awarzynski added a comment. In D123211#3435421 , @rovka wrote: > Is there actually a significant difference, besides the naming (which is easy > to change)? AFAICT the BackendAction isn't initializing anything > backend-related except very close to wher

[PATCH] D123211: [flang][driver] Add support for generating LLVM bytecode files

ekieri accepted this revision. ekieri added a comment. One nit/question inline, otherwise LGTM. Comment at: flang/lib/Frontend/FrontendActions.cpp:491 + // Create and configure `TargetMachine` + std::unique_ptr TM; + Is there a reason why use TM.reset instead

[PATCH] D123211: [flang][driver] Add support for generating LLVM bytecode files

rovka accepted this revision. rovka added a comment. This revision is now accepted and ready to land. In D123211#3433059 , @awarzynski wrote: > Thanks for taking a look Diana! > >> Why not use the BackendAction for this? > > This action only requires the

[PATCH] D123211: [flang][driver] Add support for generating LLVM bytecode files

awarzynski added a comment. Thanks for taking a look Diana! > Why not use the BackendAction for this? This action only requires the middle-end in LLVM :) The `BackendAction` will use the backend, but that's not needed here. > There seems to be a lot of shared code, up until the point where you

[PATCH] D123211: [flang][driver] Add support for generating LLVM bytecode files

awarzynski added inline comments. Comment at: flang/lib/Frontend/FrontendActions.cpp:483 + + llvm::ModulePassManager MPM; + llvm::ModuleAnalysisManager MAM; rovka wrote: > Nit: I think you can move these closer to their first use. Thanks, I thought that I _did_

[PATCH] D123211: [flang][driver] Add support for generating LLVM bytecode files

awarzynski updated this revision to Diff 420862. awarzynski added a comment. Move code a bit, add comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123211/new/ https://reviews.llvm.org/D123211 Files: clang/include/clang/Driver/Options.td

[PATCH] D123211: [flang][driver] Add support for generating LLVM bytecode files

awarzynski updated this revision to Diff 420821. awarzynski added a comment. Herald added a reviewer: sscalpone. Add missing test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123211/new/ https://reviews.llvm.org/D123211 Files: clang/include/cla

[PATCH] D123211: [flang][driver] Add support for generating LLVM bytecode files

rovka added a comment. Why not use the BackendAction for this? There seems to be a lot of shared code, up until the point where you create and use the pass manager (and in the future, when the backend switches to the new pass manager, there will be even more shared code). Co

[PATCH] D123211: [flang][driver] Add support for generating LLVM bytecode files

awarzynski created this revision. Herald added a subscriber: mgorny. Herald added projects: Flang, All. awarzynski requested review of this revision. Herald added subscribers: cfe-commits, jdoerfert. Herald added a project: clang. Support for generating LLVM BC files is added in Flang's compiler a