[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.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

Thanks all. Think I have everything so landing. Give a shout if anything else jumps out.



================
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) &&
----------------
rnk wrote:
> 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.
See the next patch? ;] It does exactly that.


================
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 &&
----------------
rnk wrote:
> Ditto for stores, I think.
Yep, also done in the next patch.


Repository:
  rL LLVM

https://reviews.llvm.org/D50680