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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

Would it be possible to add a test to show the additional dbg output, e.g. `llvm/test/Transforms/ConstraintElimination/debug.ll`?



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


================
Comment at: llvm/include/llvm/Analysis/ConstraintSystem.h:40
 public:
+  ConstraintSystem(){};
+  ConstraintSystem(DenseMap<Value *, unsigned> Value2Index)
----------------
Please run clang-format and remove the needed `;`


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


================
Comment at: llvm/lib/Analysis/ConstraintSystem.cpp:111
+  SmallVector<std::string> Names(Value2Index.size(), "");
+  for (auto &KV : Value2Index) {
+    std::string OperandName;
----------------
Can use `(auto &[V, Index] : Value2Index)`.


================
Comment at: llvm/lib/Transforms/Scalar/ConstraintElimination.cpp:639
 #ifndef NDEBUG
-static void dumpWithNames(const ConstraintSystem &CS,
-                          DenseMap<Value *, unsigned> &Value2Index) {
-  SmallVector<std::string> Names(Value2Index.size(), "");
-  for (auto &KV : Value2Index) {
-    Names[KV.second - 1] = std::string("%") + KV.first->getName().str();
-  }
-  CS.dump(Names);
-}
+static void dumpWithNames(const ConstraintSystem &CS) { CS.dump(); }
 
----------------
This function isn't needed any longer.


================
Comment at: llvm/lib/Transforms/Scalar/ConstraintElimination.cpp:642
 static void dumpWithNames(ArrayRef<int64_t> C,
                           DenseMap<Value *, unsigned> &Value2Index) {
+  ConstraintSystem CS(Value2Index);
----------------
Change to const reference and adjust name?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142618