[PATCH] D96075: [PowerPC] Exploit Prefixed Load/Stores using the refactored Load/Store Implementation

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai added a comment.

LGTM other than the missing assert. Also, please look into the interaction between prefixed instructions, pc-relative memops, paired memops and whether we do the right thing when 1/2 of those are turned on and others turned off (this can be done in a subsequent patch).



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:17019
+      unsigned ID = cast<ConstantSDNode>(Parent->getOperand(1))->getZExtValue();
+      SDValue IntrinOp = (ID == Intrinsic::ppc_vsx_lxvp) ?
+        Parent->getOperand(2) : Parent->getOperand(3);
----------------
I don't really understand what ensures that the intrinsic is one of `ppc_vsx_lxvp/ppc_vsx_stxvp`? What if it is something different entirely? It may be that we only call this function for those two now, but think about the future as well.
Even an assert would probably do here.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.h:707
+      MOF_SubtargetSPE = 1 << 25,
+      MOF_PrefixInstrs = 1 << 26
     };
----------------
I added a comment in the previous revision that `MOF_SubtargetP10` should really be something like `MOF_PrefixInstrs` but perhaps I missed something. Is there any reason we need both? Are there conditions where we want a different non-prefixed addressing mode for P10 instructions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96075