[PATCH] D45009: Reordering defs of a common user closer to the user in alphabetical order.

[Justin Bogner via Phabricator via llvm-commits]

bogner added inline comments.


================
Comment at: lib/CodeGen/MIRCanonicalizerPass.cpp:301-306
+    if (MultiUsers.find(UseToBringDefCloserTo) == MultiUsers.end()) {
+      std::vector<MachineInstr *> defs;
+      MultiUsers.insert(std::pair<MachineInstr *, std::vector<MachineInstr *>>(
+          UseToBringDefCloserTo, defs));
+    }
+    MultiUsers.find(UseToBringDefCloserTo)->second.push_back(Def);
----------------
Isn't this equivalent to MultiUsers[UseToBringDefCloserTo].push_back(Def), since you're default constructing the vector anyway?


================
Comment at: lib/CodeGen/MIRCanonicalizerPass.cpp:312-313
 
+  // sort the defs for users of multiple defs lexographically.
+  for (auto entry : MultiUsers) {
+
----------------
Stylistically, both the comment and the variable should start with capital letters. I'd probably also use "const auto &" rather than just auto here.


================
Comment at: lib/CodeGen/MIRCanonicalizerPass.cpp:321-322
+        [&](MachineInstr &MI) -> bool { return &MI == entry.first; });
+    if (UseI ==  MBB->instr_end())
+      continue;
+
----------------
What does it mean if we get here?


Repository:
  rL LLVM

https://reviews.llvm.org/D45009