[PATCH] D18093: [ScheduleDAGInstrs] Handle instructions with multiple MMOs

[Matthias Braun via llvm-commits]

MatzeB added a comment.

Some coding style suggestions:


================
Comment at: lib/CodeGen/ScheduleDAGInstrs.cpp:162
@@ -161,14 +161,3 @@
                                          const DataLayout &DL) {
-  if (!MI->hasOneMemOperand() ||
-      (!(*MI->memoperands_begin())->getValue() &&
-       !(*MI->memoperands_begin())->getPseudoValue()) ||
-      (*MI->memoperands_begin())->isVolatile())
-    return;
-
-  if (const PseudoSourceValue *PSV =
-      (*MI->memoperands_begin())->getPseudoValue()) {
-    // Function that contain tail calls don't have unique PseudoSourceValue
-    // objects. Two PseudoSourceValues might refer to the same or overlapping
-    // locations. The client code calling this function assumes this is not the
-    // case. So return a conservative answer of no known object.
-    if (MFI->hasTailCall())
+  for (auto *MMO : MI->memoperands()) {
+    if (MMO->isVolatile()) {
----------------
Please avoid auto in contexts where the type is not obvious! Using `MachineMemOperand` here is easy and if possible also add `const`.


Maybe factor all of the code inside the loop into an own function with bool return value and perform the Object.clear() cleanup when the function returned false?

================
Comment at: lib/CodeGen/ScheduleDAGInstrs.cpp:1048
@@ +1047,3 @@
+      // self-loop edge if multiple underlying objects are present.
+      for (auto &underlObj : Objs) {
+        ValueType V = underlObj.getPointer();
----------------
mcrosier wrote:
> gberry wrote:
> > mcrosier wrote:
> > > underlObj should be capitalized.
> > This is a copy of the loop above.  I think the capitalization of this (and stores_, which should probably also drop the '_') should be a separate change.
> Fair enough.
Please void auto in contexts where the type is not obvious. An obvious example would be `auto *X = new SomeType();` because the type can be seen on the right side, it is not obvious here what type the elements in Objs have.


http://reviews.llvm.org/D18093