[PATCH] D19678: Annotated-source optimization reports (a.k.a. "listing" files)

Hal Finkel via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 28 14:29:48 PDT 2016


hfinkel added a comment.

In http://reviews.llvm.org/D19678#415902, @rsmith wrote:

> You give this example:
>
> >   343     |     Loc = ConvertBackendLocation(D, Context->getSourceManager());
>
> >       I   |           ^
>
> >       I   |                                     ^
>
>
> How does this look for a case like `p->Foo()->Bar()` (where one or both of the calls are inlined)? Can we get the source location to point at the function name instead of the start of the expression to reduce the scope for ambiguity?


That does not currently work very well (I assume this needs a backend fix, but I'll check).

  $ cat /tmp/i.cpp
  void ext();
  
  struct Bar {
    void bar() { ext(); }
  };
  
  struct Foo {
    Bar *b;
  
    Bar *foo() { return b; }
  };
  
  void test(Foo *f) {
    f->foo()->bar();
  }

And we get:

  14 I   |   f->foo()->bar();

because both inlining remarks come from the backend with the same column number:

  $ clang -O3 -c -o /tmp/i.o /tmp/i.cpp -flisting -Rpass=inline
  /tmp/i.cpp:14:3: remark: _ZN3Foo3fooEv inlined into _Z4testP3Foo [-Rpass=inline]
    f->foo()->bar();
    ^
  /tmp/i.cpp:14:3: remark: _ZN3Bar3barEv inlined into _Z4testP3Foo [-Rpass=inline]


================
Comment at: lib/CodeGen/CodeGenAction.cpp:665-761
@@ +664,99 @@
+
+void BackendConsumer::GenerateListingFile() {
+  if (CodeGenOpts.ListingFile.empty())
+    return;
+
+  std::error_code EC;
+  llvm::raw_fd_ostream OS(CodeGenOpts.ListingFile, EC,
+              llvm::sys::fs::F_Text);
+  if (EC) {
+    Diags.Report(diag::err_fe_error_opening) << CodeGenOpts.ListingFile <<
+                                                EC.message();
+    return;
+  }
+
+  SourceManager &SourceMgr = Context->getSourceManager();
+  std::set<FileID> FileIDs;
+  for (auto &I : ListingInfo)
+    FileIDs.insert(SourceMgr.getFileID(I.first));
+
+  for (auto &FID : FileIDs) {
+    SourceLocation FirstLoc = SourceMgr.getLocForStartOfFile(FID);
+    OS << "< " << SourceMgr.getFilename(FirstLoc) << "\n";
+
+    auto I = ListingInfo.lower_bound(FirstLoc);
+    StringRef MB = SourceMgr.getBufferData(FID);
+    const SrcMgr::ContentCache *
+      Content = SourceMgr.getSLocEntry(FID).getFile().getContentCache();
+    unsigned LNDigits = llvm::utostr(Content->NumLines).size();
+    for (unsigned L = 0; L < Content->NumLines - 1; ++L) {
+      unsigned LStartOff = Content->SourceLineCache[L];
+      unsigned LEndOff = (L == Content->NumLines) ?
+                         Content->getSize() :
+                         Content->SourceLineCache[L + 1];
+
+      std::map<unsigned, ListingLineInfo> ColsInfo;
+      unsigned InlinedCols = 0, UnrolledCols = 0, VectorizedCols = 0;
+
+      ListingLineInfo LLI;
+      if (I != ListingInfo.end()) {
+        auto DI = SourceMgr.getDecomposedLoc(I->first);
+        while (I != ListingInfo.end() && DI.first == FID &&
+               DI.second < LStartOff) {
+          ++I;
+          if (I != ListingInfo.end())
+            DI = SourceMgr.getDecomposedLoc(I->first);
+        }
+
+        while (I != ListingInfo.end() && DI.first == FID &&
+            DI.second >= LStartOff && DI.second < LEndOff) {
+          unsigned Col = SourceMgr.getColumnNumber(FID, DI.second);
+          ColsInfo[Col] = I->second;
+          InlinedCols += I->second.Inlined.Analyzed;
+          UnrolledCols += I->second.Unrolled.Analyzed;
+          VectorizedCols += I->second.Vectorized.Analyzed;
+          LLI |= I->second;
+
+          ++I;
+          if (I != ListingInfo.end())
+            DI = SourceMgr.getDecomposedLoc(I->first);
+        }
+      }
+
+      // We try to keep the output as concise as possible. If only one thing on
+      // a given line could have been inlined, vectorized, etc. then we can put
+      // the marker on the source line itself. If there are multiple options
+      // then we want to distinguish them by placing the marker for each
+      // transformation on a separate line following the source line. When we
+      // do this, we use a '^' character to point to the appropriate column in
+      // the source line.
+
+      OS << llvm::format_decimal(L + 1, LNDigits) << " ";
+      OS << (LLI.Inlined.Transformed && InlinedCols < 2 ? "I" : " ");
+      OS << (LLI.Unrolled.Transformed && UnrolledCols < 2 ? "U" : " ");
+      OS << (LLI.Vectorized.Transformed && VectorizedCols < 2 ? "V" : " ");
+
+      OS << " | " << MB.slice(LStartOff, LEndOff);
+
+      for (auto &J : ColsInfo) {
+        if ((J.second.Inlined.Transformed && InlinedCols > 1) ||
+            (J.second.Unrolled.Transformed && UnrolledCols > 1) ||
+            (J.second.Vectorized.Transformed && VectorizedCols > 1)) {
+          OS << std::string(LNDigits + 1, ' ');
+          OS << (J.second.Inlined.Transformed &&
+                 InlinedCols > 1 ? "I" : " ");
+          OS << (J.second.Unrolled.Transformed &&
+                 UnrolledCols > 1 ? "U" : " ");
+          OS << (J.second.Vectorized.Transformed &&
+                 VectorizedCols > 1 ? "V" : " ");
+
+          OS << " | " << std::string(J.first - 1, ' ') << "^\n";
+        }
+      }
+
+      if (LEndOff == Content->getSize())
+        OS << "\n";
+    }
+  }
+}
+
----------------
rsmith wrote:
> I'd like this to be factored out and moved somewhere more appropriate (such as Frontend). It seems appropriate for CodeGen to generate the data structure here, but it should not be deciding how to format the report nor doing file IO to put it somewhere.
> 
> I would hope that we can combine this report information with the static analyzer's existing support for generating syntax-highlighted, annotated source code as HTML as a future extension.
> I'd like this to be factored out and moved somewhere more appropriate (such as Frontend). It seems appropriate for CodeGen to generate the data structure here, but it should not be deciding how to format the report nor doing file IO to put it somewhere.

Makes sense.

> I would hope that we can combine this report information with the static analyzer's existing support for generating syntax-highlighted, annotated source code as HTML as a future extension.

I like this idea.



http://reviews.llvm.org/D19678





More information about the cfe-commits mailing list