[PATCH] D93370: [PowerPC] Add new infrastructure to select load/store instructions, update P8/P9 load/store patterns.

[Amy Kwan via Phabricator via llvm-commits]

amyk added a comment.

@steven.zhang The idea is that this patch should be NFC. All existing load/store test cases should pass with this refactoring. I do think there should be more tests added, perhaps in a follow up patch. What do you think?



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:2502
+/// codegen it as an add (for better address arithmetic).
+bool provablyDisjointOr(SelectionDAG &DAG, const SDValue &N) {
+  if (N.getOpcode() != ISD::OR)
----------------
lkail wrote:
> Should it be a `static` function?
Good point. I will fix this.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.h:717
     const PPCSubtarget &Subtarget;
+    std::map<PPC::AddrMode, SmallVector<unsigned, 16>> AddrModesMap;
+    void initializeAddrModeMap();
----------------
Add comments to describe the functions.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstr64Bit.td:1065
                     [(set i64:$rD,
-                          (aligned4sextloadi32 iaddrX4:$src))]>, isPPC64,
+                          (sextloadi32 dsfaddr:$src))]>, isPPC64,
                     PPC970_DGroup_Cracked;
----------------
steven.zhang wrote:
> Not sure if this is handled correctly as you are removing the align restrict for LWA. Technical speaking, we need to remove all such kind of alignment restrict in the load/store as far as we did some analysis in the source code. But I notice that we are removing the align restrict here but still keep it for LD. 
You're right; I meant to remove the `align`/`unalign` in the other `LD`,`LWA` patterns, as well, as the new load/store infrastructure is meant to use load/sextload/zextload and compute alignment based on the flags. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93370