rovka accepted this revision. rovka added a comment. This revision is now accepted and ready to land.
In D123211#3433059 <https://reviews.llvm.org/D123211#3433059>, @awarzynski wrote: > 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. 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. >> There seems to be a lot of shared code, up until the point where you create >> and use the pass manager > > This is the bit that's shared: > > CompilerInstance &ci = this->instance(); > // Generate an LLVM module if it's not already present (it will already be > // present if the input file is an LLVM IR/BC file). > if (!llvmModule) > GenerateLLVMIR(); > > // Create and configure `Target` > std::string error; > std::string theTriple = llvmModule->getTargetTriple(); > const llvm::Target *theTarget = > llvm::TargetRegistry::lookupTarget(theTriple, error); > assert(theTarget && "Failed to create Target"); > > // Create and configure `TargetMachine` > std::unique_ptr<llvm::TargetMachine> TM; > > TM.reset(theTarget->createTargetMachine(theTriple, /*CPU=*/"", > /*Features=*/"", llvm::TargetOptions(), llvm::None)); > assert(TM && "Failed to create TargetMachine"); > llvmModule->setDataLayout(TM->createDataLayout()); > > I wouldn't say it's "a lot", but probably enough for a dedicated method. I've > been conservative with regard to sharing code as I anticipate things to > change in the future (I expect `-O{0|1|2|3|s|z}` to complicate the logic a > bit). 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. >> (and in the future, when the backend switches to the new pass manager, there >> will be even more shared code). > > I'm not sure about the timelines for this. And even then, the logic might be > quite different (again, middle-end vs back-end). 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. No other objections from my side, thanks :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123211/new/ https://reviews.llvm.org/D123211 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
