[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