[PATCH] Remove all MMOs from memory operations when tail merging.

[Chad Rosier]

Hi hfinkel, ahatanak, qcolombet,

All,
When tail merging MBBs it's necessary to remove all MMOs from memory operations to ensures later passes conservatively compute dependencies.

A more robust solution would be to add multiple MMOs from the duplicate MIs to the new MI.  Currently ScheduleDAGInstrs.cpp ignores all MMOs on instructions with multiple MMOs, so the attached patch is equivalent for the time being.  If this patch is approved, I'll file a new PR suggesting the potential enhancement.

Just a few questions:
1. Does the new clearMemRefs() API need to free any memory?
2. I couldn't derive a simple test case.  Does anyone have a suggestion?

Initial discussion here:
http://lists.cs.uiuc.edu/pipermail/llvmdev/2015-February/082525.html
http://lists.cs.uiuc.edu/pipermail/llvmdev/2015-February/082526.html

http://reviews.llvm.org/D7769

Files:
  include/llvm/CodeGen/MachineInstr.h
  lib/CodeGen/BranchFolding.cpp

Index: include/llvm/CodeGen/MachineInstr.h
===================================================================
--- include/llvm/CodeGen/MachineInstr.h
+++ include/llvm/CodeGen/MachineInstr.h
@@ -1170,6 +1170,12 @@
     assert(NumMemRefs == NewMemRefsEnd - NewMemRefs && "Too many memrefs");
   }
 
+  /// clearMemRefs - Clear this MachineInstr's memory reference descriptor list.
+  void clearMemRefs() {
+    MemRefs = nullptr;
+    NumMemRefs = 0;
+  }
+
 private:
   /// getRegInfo - If this instruction is embedded into a MachineFunction,
   /// return the MachineRegisterInfo object for the current function, otherwise
Index: lib/CodeGen/BranchFolding.cpp
===================================================================
--- lib/CodeGen/BranchFolding.cpp
+++ lib/CodeGen/BranchFolding.cpp
@@ -727,6 +727,14 @@
   return true;
 }
 
+static void removeMMOsFromMemoryOperations(MachineBasicBlock &MBB) {
+  // Remove all MMOs from memory operations.  This ensures later passes
+  // conservatively compute dependencies.
+  for (auto &MII : MBB)
+    if (MII.mayLoad() || MII.mayStore())
+      MII.clearMemRefs();
+}
+
 // See if any of the blocks in MergePotentials (which all have a common single
 // successor, or all have no successor) can be tail-merged.  If there is a
 // successor, any blocks in MergePotentials that are not tail-merged and
@@ -828,6 +836,9 @@
 
     MachineBasicBlock *MBB = SameTails[commonTailIndex].getBlock();
 
+    // Remove all MMOs from memory operations.
+    removeMMOsFromMemoryOperations(*MBB);
+
     // Recompute commont tail MBB's edge weights and block frequency.
     setCommonTailEdgeWeights(*MBB);

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D7769.20338.patch
Type: text/x-patch
Size: 1650 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150219/6d545465/attachment.bin>