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

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

In D123211#3435421 <https://reviews.llvm.org/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 where it's using it, so that can happen inside a switch/if branch.

Currently the driver does the bare minimum to generate machine code, hence in practice both actions are near identical. I expect this to change, but I might be wrong :)

> I was thinking also about the code for generating the output file, which can be folded into the switch from BackendAction. If you consider that too, it becomes a very large percentage of EmitLLVMBitcodeAction::ExecuteAction that can be shared.

Ack!

> Ok. IMO the template method pattern would work well here (or less formally, just a simple switch to the same effect), but I can understand if you think it's premature to go that route.

Definitely not objecting code re-use! :) But I would like to wait for a few patches for optimisation pipelines to land first. And if anyone feels differently and sends a patch, I'd be alright with that.



================
Comment at: flang/lib/Frontend/FrontendActions.cpp:491
+  // Create and configure `TargetMachine`
+  std::unique_ptr<llvm::TargetMachine> TM;
+
----------------
ekieri wrote:
> Is there a reason why use TM.reset instead of initialising TM like you do with os below?
Good question!

Note that [[ https://github.com/llvm/llvm-project/blob/3d0e0e1027203fe5e89104ad81ee7bb53e525f95/llvm/include/llvm/MC/TargetRegistry.h#L446-L452 | createTargetMachine ]] that I use below returns a plain pointer. [[ https://github.com/llvm/llvm-project/blob/3d0e0e1027203fe5e89104ad81ee7bb53e525f95/clang/include/clang/Frontend/CompilerInstance.h#L703-L706 | createDefaultOutputFile ]] that I use for initialising `os` below returns `std::unique_ptr<raw_pwrite_stream>`. IIUC, I can only [[ https://en.cppreference.com/w/cpp/memory/unique_ptr/reset | reset ]] a `unique_ptr` (like `TM`) with a plain pointer.

Have I missed anything?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123211