[PATCH] D33222: [LegacyPassManager] Remove TargetMachine constructors

[Matthias Braun via Phabricator via llvm-commits]

MatzeB accepted this revision.
MatzeB added a comment.
This revision is now accepted and ready to land.

Thanks, this turned out to be a nice code simplification!
LGTM with nitpicks (including Chandlers comments) addressed.



================
Comment at: lib/CodeGen/TargetPassConfig.cpp:318
   : ImmutablePass(ID), PM(nullptr) {
   llvm_unreachable("TargetPassConfig should not be constructed on-the-fly");
 }
----------------
I assume this is the message people will see when they schedule a codegen pass without a target machine now. Could you maybe improve the error message to be more helpful?
"Trying to construct TargetPassConfig without target machine. Scheduling a CodeGen pass without a target triple set?"


================
Comment at: lib/Target/Mips/MipsModuleISelDAGToDAG.cpp:40-41
   DEBUG(errs() << "In MipsModuleDAGToDAGISel::runMachineFunction\n");
+  auto &TM = const_cast<MipsTargetMachine &>(
+      static_cast<const MipsTargetMachine &>(MF.getTarget()));
   TM.resetSubtarget(&MF);
----------------
chandlerc wrote:
> This is horrible. =/
> 
> I think the `MipsTargetMachine` should directly expose a const-qualified `resetSubtarget` method, as it is already doing crazy const-casting... Or it should have some other way of doing this.
> 
> I might just leave the original code for this part of the Mips backend rather than trying to make it clean in the same way.
I assume you can address this by getting a non-const TargetMachine from the TargetPassConfig.


================
Comment at: lib/Target/Sparc/LeonPasses.h:35
 protected:
-  LEONMachineFunctionPass(TargetMachine &tm, char &ID);
+  LEONMachineFunctionPass();
   LEONMachineFunctionPass(char &ID);
----------------
It looks like this constructor can be removed completely now. Could you try?


https://reviews.llvm.org/D33222