[PATCH] D50680: [SDAG] Remove the reliance on MI's allocation strategy for `MachineMemOperand` pointers attached to `MachineSDNodes` and instead have the `SelectionDAG` fully manage the memory for this array.

Reid Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 14 15:57:25 PDT 2018


rnk accepted this revision.
rnk added a comment.

+1



================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:5521-5522
-              MachineInstr::mmo_iterator> MMOs =
-      MF.extractLoadMemRefs(cast<MachineSDNode>(N)->memoperands_begin(),
-                            cast<MachineSDNode>(N)->memoperands_end());
-    if (!(*MMOs.first) &&
----------------
There is only one other call site of extractLoadMemRefs, and its only called from X86InstrInfo.cpp. Can you migrate the other call site over to your extractLoadMMOs helper and delete MachineFunction::extractLoadMemRefs? Now that we can get an `ArrayRef<MachineMemOperand*>` from both MachineSDNodes and MachineInstr, we should be able to share.


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:5631
     AddrOps.push_back(Chain);
-    std::pair<MachineInstr::mmo_iterator,
-              MachineInstr::mmo_iterator> MMOs =
-      MF.extractStoreMemRefs(cast<MachineSDNode>(N)->memoperands_begin(),
-                             cast<MachineSDNode>(N)->memoperands_end());
-    if (!(*MMOs.first) &&
-        RC == &X86::VR128RegClass &&
+    auto MMOs = extractStoreMMOs(cast<MachineSDNode>(N)->memoperands(), MF);
+    if (MMOs.empty() && RC == &X86::VR128RegClass &&
----------------
Ditto for stores, I think.


Repository:
  rL LLVM

https://reviews.llvm.org/D50680





More information about the llvm-commits mailing list