[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:
> > shchenz wrote:
> > > arsenm wrote:
> > > > arsenm wrote:
> > > > > 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 
> > > > The tests aren't covering these various use checks 
> > > Sorry, I meant to add more cases, but seems now there are not too many generic opcodes support on PPC target for globalisel, for example `G_FPTOSI/G_SITOFP`...
> > Should add a few simple ones in a pre-commit to use in tests here
> pre-commit cases will crash because there is no load/store support for PPC GISEL . Do you mean I pre-commit these crash ones guarded with `not --crash`?
No, I mean do a simple patch to add support for a few more FP operations. You have working arg and return, so it's not dependent on load/store first


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134792