[PATCH] Improve debug message of the layout pass.

Shankar Kalpathi Easwaran shankarke at gmail.com
Thu Oct 17 20:08:59 PDT 2013



================
Comment at: lib/Passes/LayoutPass.cpp:127
@@ +126,3 @@
+  DEBUG({
+    StringRef comp = result ? "<" : ">";
+    llvm::dbgs() << "Layout: '" << left->name() << "' " << comp << " '" << right->name()
----------------
Rui Ueyama wrote:
> Shankar Kalpathi Easwaran wrote:
> > equal atoms also would be shown as > ?
> Look at the last line of compare(). No two atoms will be equal.
I mentioned this comment because of lines 43-46.

================
Comment at: lib/Passes/LayoutPass.cpp:40-42
@@ -30,5 +39,5 @@
 /// f) Sorts atoms on how they appear within the File
-bool LayoutPass::CompareAtoms::operator()(const DefinedAtom *left,
-                                          const DefinedAtom *right) const {
-  DEBUG(llvm::dbgs() << "Sorting " << left->name() << " " << right->name() << "\n");
-  if (left == right)
+bool LayoutPass::CompareAtoms::compare(
+    const DefinedAtom *left, const DefinedAtom *right, std::string &reason)
+    const {
+  if (left == right) {
----------------
Rui Ueyama wrote:
> Shankar Kalpathi Easwaran wrote:
> > Looks like the prototype became unreadable now. can we use the prototype like earlier ?
> This is a new function and the operator()'s prototype remains the same.
If the definition was like this, feel its more readable.


> bool LayoutPass::CompareAtoms::compare(const DefinedAtom *left,
>                                                                const DefinedAtom *right, 
>                                                                std::string &reason) const {
> }


compared to the current.

I dont want to nit pick on this, but a suggestion.

PS : Phabricator is spoiling the indentation :)


http://llvm-reviews.chandlerc.com/D1964

COMMIT
  http://llvm-reviews.chandlerc.com/rL192941



More information about the llvm-commits mailing list