[PATCH] D134792: [PowerPC][GISel] support 64 bit load/store

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/GISel/PPCInstructionSelector.cpp:264-265
+    return false;
+  case TargetOpcode::G_LOAD:
+  case TargetOpcode::G_STORE: {
+    GLoadStore &LdSt = cast<GLoadStore>(I);
----------------
What's the problem with importing the tablegen patterns? This looks pretty simple?


================
Comment at: llvm/lib/Target/PowerPC/GISel/PPCInstructionSelector.cpp:289
+      // FIXME: optimize load/store with some specific address patterns.
+      MachineIRBuilder MIB(I);
+      auto NewInst = MIB.buildInstr(NewOpc, {}, {}, I.getFlags());
----------------
Shouldn't create one off MachineIRBuilders


================
Comment at: llvm/lib/Target/PowerPC/GISel/PPCRegisterBankInfo.cpp:133-134
+    Register VReg = MI.getOperand(0).getReg();
+    if (!VReg)
+      break;
+    MachineInstr *DefMI = MRI.getVRegDef(VReg);
----------------
This cannot happen


================
Comment at: llvm/lib/Target/PowerPC/GISel/PPCRegisterBankInfo.cpp:153-155
+/// FIXME: this is copied from target AArch64. Needs some code refactor here to
+/// put this function in class RegisterBankInfo.
+static bool isPreISelGenericFloatingPointOpcode(unsigned Opc) {
----------------
Should put in utils 


================
Comment at: llvm/lib/Target/PowerPC/GISel/PPCRegisterBankInfo.cpp:168
+  case TargetOpcode::G_FNEARBYINT:
+  case TargetOpcode::G_FNEG:
+  case TargetOpcode::G_FCOS:
----------------
I wouldn't really call FNEG or FABS FP opcodes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134792