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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm requested changes to this revision.
arsenm added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/Target/PowerPC/GISel/PPCRegisterBankInfo.cpp:123
+    // Check if that load feeds fp instructions.
+    if (any_of(MRI.use_nodbg_instructions(MI.getOperand(0).getReg()),
+               [&](const MachineInstr &UseMI) {
----------------
I think the intent is the call here shouldn't need to look at uses


================
Comment at: llvm/lib/Target/PowerPC/GISel/PPCRegisterBankInfo.cpp:143
+    // Check if the store is fed by fp instructions.
+    MachineInstr *DefMI = MRI.getVRegDef(MI.getOperand(0).getReg());
+    if (onlyDefinesFP(*DefMI, MRI, TRI))
----------------
This call doesn't make any sense, you're just getting a pointer to MI with more steps 


================
Comment at: llvm/lib/Target/PowerPC/GISel/PPCRegisterBankInfo.cpp:168
+  case TargetOpcode::G_FNEARBYINT:
+  case TargetOpcode::G_FNEG:
+  case TargetOpcode::G_FCOS:
----------------
shchenz wrote:
> arsenm wrote:
> > I wouldn't really call FNEG or FABS FP opcodes
> Sorry, I do not quite understand. Do you mean we should not add `G_FNEG` and `G_FABS` here? But these two opcodes are floating pointer operations, with one floating point input and one floating point output?
They don't really have floating point semantics. They're non-canonicalizing bit operations, but maybe it makes sense for PPC instructions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134792