[PPC][GlobalISel] Add initial GlobalIsel infrastructure

[kbarton]

Summary

This adds the initial GlobalISel skeleton for PowerPC. It can only run
ir-translator and legalizer for ret void.

This is largely based on the initial GlobalISel patch for RISCV
(https://reviews.llvm.org/D65219).

[arsenm]

llvm/lib/Target/PowerPC/PPC.h

88–89

These should be const?

llvm/lib/Target/PowerPC/PPCInstructionSelector.cpp

39

I'm pretty sure you don't need these and all the other places that override this are dead code

[kbarton]

Add const to all parameters of createPPCInstructionSelector.

[kbarton]

Refreshing diff.

[kbarton]

Refreshing diff

[kbarton]

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.

llvm/lib/Target/PowerPC/PPC.h

88–89

Yup, thanks for catching this. Fixed.

llvm/lib/Target/PowerPC/PPCInstructionSelector.cpp

39

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?

[tschuett]

AArch64 has a sub-directory for GlobalISel related things:
https://reviews.llvm.org/D81116

[madhur13490]

llvm/lib/Target/PowerPC/PPCCallLowering.cpp

42 ↗

(On Diff #275412)

Why not do "return VRegs.empty()"? You can add a comment if it needs readability.

llvm/lib/Target/PowerPC/PPCInstructionSelector.cpp

23

May be ppc-gisel better?

[tschuett]

llvm/lib/Target/PowerPC/PPCLegalizerInfo.h

21 ↗

(On Diff #275412)

Legalizer or register banks?

[arsenm]

llvm/lib/Target/PowerPC/PPCInstructionSelector.cpp

39

Oh right, this isn't the direct pass. I think manual getName overrides are dead code on passes and set by the INITIALIZE_PASS* macros

[kbarton]

• Address review comments - minor clean in PPC::lowerFormalArguments and update comments.

[kbarton]

In D83100#2130726, @tschuett wrote:

AArch64 has a sub-directory for GlobalISel related things:
https://reviews.llvm.org/D81116

I like this idea. I will do the refactoring in a subsequent commit and update the patch, so if others do not like it it's easy to undo.

llvm/lib/Target/PowerPC/PPCInstructionSelector.cpp

23

Yes, this is a better idea. Will change it.

39

Yes, that could be. I haven't looked at the implementation for how it works on passes.

Are you OK marking this as done?

llvm/lib/Target/PowerPC/PPCLegalizerInfo.h

21 ↗

(On Diff #275412)

Good catch. Updated to legalizer.

[arsenm]

In D83100#2134028, @kbarton wrote:

In D83100#2130726, @tschuett wrote:

AArch64 has a sub-directory for GlobalISel related things:
https://reviews.llvm.org/D81116

I like this idea. I will do the refactoring in a subsequent commit and update the patch, so if others do not like it it's easy to undo.

I don't particularly like the AArch64 split. There isn't an entirely clean break between globalisel parts and the rest of the backend. The other target subdirectories are used for places where there's a separate library

[kbarton]

• Put all new GlobalISel related files into new GISel subdirectory.

[tschuett]

Pardon my cmake, but don't you need a CMakeLists.txt in the GISel sub-directory?

[kbarton]

In D83100#2134249, @tschuett wrote:

Pardon my cmake, but don't you need a CMakeLists.txt in the GISel sub-directory?

I thought I would too, but I added GISel/ prefix to the CMake in the PowerPC directory, which works.
This is the way it was done in the Aarch64 patch that did the refactoring.

That said, I am by no means a cmake expert, so if having a separate CMakeLists.txt file in the GISel directory is "better", I can add one there also.

[arsenm]

In D83100#2136392, @kbarton wrote:

In D83100#2134249, @tschuett wrote:

Pardon my cmake, but don't you need a CMakeLists.txt in the GISel sub-directory?

I thought I would too, but I added GISel/ prefix to the CMake in the PowerPC directory, which works.
This is the way it was done in the Aarch64 patch that did the refactoring.

That said, I am by no means a cmake expert, so if having a separate CMakeLists.txt file in the GISel directory is "better", I can add one there also.

It's not a separate library, so it's not better. Things are not layered to make this possible

[kbarton]

Fix warnings from clang-tidy and clang-format and rebase to latest version of master.

[arsenm]

llvm/lib/Target/PowerPC/PPCSubtarget.h

166 ↗

(On Diff #284039)

InlineAsmLowering is another boilerplate field, but you can also leave that for later

[kbarton]

• Add const to all parameters of createPPCInstructionSelector.
• Address review comments - minor clean in PPC::lowerFormalArguments and update comments.
• Put all new GlobalISel related files into new GISel subdirectory.
• Fix warnings from clang-tidy and clang-format.
• Remove unnecessary whitespace.

[arsenm]

llvm/test/CodeGen/PowerPC/GlobalISel/legalize-ret.mir

1 ↗

(On Diff #286131)

Don't need -global-isel if directly running MIR passes

llvm/lib/Target/PowerPC/PPC.h

#endif
   void initializePPCMIPeepholePass(PassRegistry&);
 
   extern char &PPCVSXFMAMutateID;
 
   ModulePass *createPPCLowerMASSVEntriesPass();
   void initializePPCLowerMASSVEntriesPass(PassRegistry &);
   extern char &PPCLowerMASSVEntriesID;
 
-  InstructionSelector *createPPCInstructionSelector(const PPCTargetMachine &,
-                                                    PPCSubtarget &,
-                                                    PPCRegisterBankInfo &);
+  InstructionSelector *
+  createPPCInstructionSelector(const PPCTargetMachine &, const PPCSubtarget &,
+                               const PPCRegisterBankInfo &);
   namespace PPCII {
 
   /// Target Operand Flag enum.
   enum TOF {
     //===------------------------------------------------------------------===//
     // PPC Specific MachineOperand flags.
     MO_NO_FLAG,
 

llvm/lib/Target/PowerPC/PPCInstructionSelector.cpp

 #include "PPCSubtarget.h"
 #include "PPCTargetMachine.h"
 #include "llvm/CodeGen/GlobalISel/InstructionSelector.h"
 #include "llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/IR/IntrinsicsPowerPC.h"
 #include "llvm/Support/Debug.h"
 
 #define DEBUG_TYPE "ppc-isel"
 
 using namespace llvm;
 
 namespace {
 
 #define GET_GLOBALISEL_PREDICATE_BITSET
 #include "PPCGenGlobalISel.inc"
 #undef GET_GLOBALISEL_PREDICATE_BITSET
 
 class PPCInstructionSelector : public InstructionSelector {
 public:
   PPCInstructionSelector(const PPCTargetMachine &TM, const PPCSubtarget &STI,
                          const PPCRegisterBankInfo &RBI);
 
   bool select(MachineInstr &I) override;
   static const char *getName() { return DEBUG_TYPE; }
 
 private:
   /// tblgen generated 'select' implementation that is used as the initial
   /// selector for the patterns that do not require complex C++.
   bool selectImpl(MachineInstr &I, CodeGenCoverage &CoverageInfo) const;
 
   const PPCInstrInfo &TII;
   const PPCRegisterInfo &TRI;

 
 bool PPCInstructionSelector::select(MachineInstr &I) {
   if (selectImpl(I, *CoverageInfo))
     return true;
   return false;
 }
 
 namespace llvm {
-InstructionSelector *createPPCInstructionSelector(const PPCTargetMachine &TM,
-                                                  PPCSubtarget &Subtarget,
-                                                  PPCRegisterBankInfo &RBI) {
+InstructionSelector *
+createPPCInstructionSelector(const PPCTargetMachine &TM,
+                             const PPCSubtarget &Subtarget,
+                             const PPCRegisterBankInfo &RBI) {
   return new PPCInstructionSelector(TM, Subtarget, RBI);
 }
 } // end namespace llvm