[PATCH] D83100: [PPC][GlobalISel] Add initial GlobalIsel infrastructure

[Kit Barton via Phabricator via llvm-commits]

kbarton marked 2 inline comments as done.
kbarton added a comment.

I had some problems refreshing the patch using arcanist. I *think* I have fixed everything now, but if things look odd please let me know.



================
Comment at: llvm/lib/Target/PowerPC/PPC.h:88-89
+  InstructionSelector *createPPCInstructionSelector(const PPCTargetMachine &,
+                                                    PPCSubtarget &,
+                                                    PPCRegisterBankInfo &);
   namespace PPCII {
----------------
arsenm wrote:
> These should be const?
Yup, thanks for catching this. Fixed. 


================
Comment at: llvm/lib/Target/PowerPC/PPCInstructionSelector.cpp:39
+  bool select(MachineInstr &I) override;
+  static const char *getName() { return DEBUG_TYPE; }
+
----------------
arsenm wrote:
> I'm pretty sure you don't need these and all the other places that override this are dead code
I don't follow this. 
Both select and getName seem to be required - getName is needed by the base InstructionSelector implementation in GlobalISel; select is needed by the PPCGenGlobalISel.inc file generated below.

It is entirely possible I'm doing something incorrect though. Could you explain some more?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83100