[PATCH] D142618: [ConstraintElimination] Change debug output to display variable names.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/include/llvm/Analysis/ConstraintSystem.h:42
+  ConstraintSystem(const DenseMap<Value *, unsigned> Value2Index)
+      : Value2Index(Value2Index){};
   bool addVariableRow(ArrayRef<int64_t> R) {
----------------
newline before the next function.


================
Comment at: llvm/include/llvm/Analysis/ConstraintSystem.h:42
+  ConstraintSystem(const DenseMap<Value *, unsigned> Value2Index)
+      : Value2Index(Value2Index){};
   bool addVariableRow(ArrayRef<int64_t> R) {
----------------
fhahn wrote:
> newline before the next function.
`;` here also needs removed and reformatting. Please run clang-format.


================
Comment at: llvm/include/llvm/Analysis/ConstraintSystem.h:102
 
   /// Print the constraints in the system, using \p Names as variable names.
+  void dump() const;
----------------
The bit about using `Names` needs to be removed.


================
Comment at: llvm/include/llvm/Analysis/ConstraintSystem.h:37
+  /// get list of variable names from Value2Index map
+  SmallVector<std::string> getVarNamesList() const;
 
----------------
fhahn wrote:
> capitalize first letter of sentence and period at end of sentence. Newline before comment
still needs a newline before the comment.


================
Comment at: llvm/include/llvm/Analysis/ConstraintSystem.h:41
+  ConstraintSystem(){};
+  ConstraintSystem(DenseMap<Value *, unsigned> Value2Index)
+      : Value2Index(Value2Index){};
----------------
fhahn wrote:
> Pass argument as const reference?
not just const, const reference, i.e. `const DenseMap<Value *, unsigned> &Value2Index`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142618