Page MenuHomePhabricator

[Minor patch] Fix IR Module Printing
Needs RevisionPublic

Authored by tyb0807 on Apr 18 2018, 8:06 AM.

Details

Summary

Add a CRLF character before printing an IR Module in IRPrintingPasses

Diff Detail

Repository
rL LLVM

Event Timeline

tyb0807 created this revision.Apr 18 2018, 8:06 AM

Update with the context

So for now, with -print-after-all or -print-before-all options in opt, we have the banner and the first line of the IR Module on a same line, e.g:

*** IR Dump After Force set function attributes *** ; ModuleID = '<stdin>'
source_filename = "main.c"
...

This PR should fix this and we'll have:

*** IR Dump After Force set function attributes ***
; ModuleID = '<stdin>'
source_filename = "main.c"
...

svn blame leads me to this

xbolva00 accepted this revision.Apr 23 2018, 6:01 AM
This revision is now accepted and ready to land.Apr 23 2018, 6:01 AM
dexonsmith requested changes to this revision.Apr 23 2018, 6:27 AM

That doesn’t seem like the right place for the newline; instead, it should be wherever the asterisks were printed.

Also, please add a test.

This revision now requires changes to proceed.Apr 23 2018, 6:27 AM

Hmmm, the problem is in AssemblyWriter::printFunction(), we already print a newline character at the start of the function, so in fact this issue only raises when we print ModulePasses. Removing the newline character printing in AssemblyWriter::printFunction() will lead to this:

@x = common dso_local global i32 0, align 4
@y = common dso_local global i32 0, align 4
@z = common dso_local global i32 0, align 4
; Function Attrs: noinline nounwind uwtable

instead of this:

@x = common dso_local global i32 0, align 4
@y = common dso_local global i32 0, align 4
@z = common dso_local global i32 0, align 4

; Function Attrs: noinline nounwind uwtable

What would be the best way to fix this?

Another high-level note (besides adding tests): please resubmit the patch with full context (e.g., git diff -U9999999 HEAD^..).

What would be the best way to fix this?

I suspect you can change the appropriate callers of printFunction to add a newline in between functions. E.g., something like this would work:

bool IsFirst = true;
for (auto &F : functions()) {
  if (IsFirst)
    IsFirst = false;
  else
    OS << "\n";
  printFunction(F);
}
xbolva00 requested changes to this revision.Apr 25 2018, 1:38 PM
xbolva00 resigned from this revision.Apr 30 2018, 4:12 AM