[PATCH] D66029: llvm-canon

[Mandeep Singh Grang via Phabricator via llvm-commits]

mgrang added inline comments.


================
Comment at: tools/llvm-canon/canonicalizer.cpp:164
+        // This instruction is commutative, sort operands.
+        std::sort(Operands.begin(), Operands.end());
+    }
----------------
Please use the range-based llvm::sort(Operands) instead of std::sort.

See https://llvm.org/docs/CodingStandards.html#beware-of-non-deterministic-sorting-order-of-equal-elements


================
Comment at: tools/llvm-canon/canonicalizer.cpp:256
+        // This instruction is commutative, sort operands.
+        std::sort(Operands.begin(), Operands.end());
+    }
----------------
Use llvm::sort.
Braces are not needed for single statement if's.


================
Comment at: tools/llvm-canon/canonicalizer.cpp:285
+        // This instruction is commutative, sort operand opcodes for hashing.
+        std::sort(OperandsOpcodes.begin(), OperandsOpcodes.end());
+    }
----------------
Ditto.


================
Comment at: tools/llvm-canon/canonicalizer.cpp:366
+        // This instruction is commutative, sort operands.
+        std::sort(Operands.begin(), Operands.end());
+    }
----------------
Ditto.


================
Comment at: tools/llvm-canon/canonicalizer.cpp:467
+    // Sort operands.
+    std::sort(Operands.begin(), Operands.end(), []
+    (const std::pair<std::string, Value*> &lhs, const std::pair<std::string, Value*> &rhs)
----------------
Use llvm::sort.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66029/new/

https://reviews.llvm.org/D66029