[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/PPCRegisterBankInfo.cpp:123
+    // Check if that load feeds fp instructions.
+    if (any_of(MRI.use_nodbg_instructions(MI.getOperand(0).getReg()),
+               [&](const MachineInstr &UseMI) {
----------------
shchenz wrote:
> arsenm wrote:
> > I think the intent is the call here shouldn't need to look at uses
> Thanks for review.
> 
> I am a little confused by this comment. Do we have any new design for the bank selection for G_LOAD/G_STORE. As we don't have G_FLOAD for float type, to me if we need to distinguish float load and fix point load, we have to go through the G_LOAD's users to decide the load instruction's register bank. On PPC, we have fix point load instructions ld/lwa(and other for other width), and we also have instructions lfd/lfs for float point load.
> 
> This is the same handling with AArch4 target for G_LOAD/G_STORE node. See https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp#L779-L792
I guess I thought the point of all the mapping costs were to let the pass look at the uses. I haven't looked that closely at this usage of regbankselect 


================
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))
----------------
shchenz wrote:
> arsenm wrote:
> > This call doesn't make any sense, you're just getting a pointer to MI with more steps 
> Similar with G_LOAD. With current Generic MIR infra, on PPC, we have to know what instruction defines the operand 0 of the MI, float point or fixed point. Operand 0 is the content stored to the memory, right?
Operand 0 is the result operand. Operand 1 is the stored value 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134792