[PATCH] Improve debug message of the layout pass.

Rui Ueyama ruiu at google.com
Thu Oct 17 20:17:37 PDT 2013



================
Comment at: lib/Passes/LayoutPass.cpp:127
@@ +126,3 @@
+  DEBUG({
+    StringRef comp = result ? "<" : ">";
+    llvm::dbgs() << "Layout: '" << left->name() << "' " << comp << " '" << right->name()
----------------
Shankar Kalpathi Easwaran wrote:
> 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.
Good point, >= would probably better.

================
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) {
----------------
Shankar Kalpathi Easwaran wrote:
> 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 :)
I didn't get that you were saying about the indentation but function prototype (parameters and types)... The indentation you wrote was exactly clang-format did, so it's already fixed.


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

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



More information about the llvm-commits mailing list