[PATCH] D22655: Avoid dsymutil calls to getFileNameByIndex

Frederic Riss via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 21 18:24:57 PDT 2016


friss accepted this revision.
friss added a comment.
This revision is now accepted and ready to land.

This LGTM if you agree with my comments. Thanks for the nice speed boost!


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFDebugLine.h:191-192
@@ -190,1 +190,4 @@
 
+    bool hasFileNameByIndex(uint64_t FileIndex,
+                            DILineInfoSpecifier::FileLineInfoKind Kind) const;
+
----------------
I know you modeled the name on the function below, but it reads strange to me. How about just
```
 bool hasFileAtIndex(uint64_t FileIndex) const;
```
(ie also drop the Kind parameter, I don't think it makes sense to test it here.)

================
Comment at: lib/DebugInfo/DWARF/DWARFDebugLine.cpp:630-633
@@ -631,3 +629,6 @@
+                                              FileLineInfoKind Kind) const {
   if (FileIndex == 0 || FileIndex > Prologue.FileNames.size() ||
       Kind == FileLineInfoKind::None)
     return false;
+  return true;
+}
----------------
I like
```
return FileIndex != 0 && FileIndex <= Prologue.FileNames.size() ;
```
better (if you drop the Kind like I said above)

================
Comment at: lib/DebugInfo/DWARF/DWARFDebugLine.cpp:641
@@ +640,3 @@
+                                              std::string &Result) const {
+  if (!hasFileNameByIndex(FileIndex, Kind))
+    return false;
----------------
Then this becomes:
```
if (Kind == FileLineInfoKind::None || !hasFileAtIndex(FileIndex))
```


Repository:
  rL LLVM

https://reviews.llvm.org/D22655





More information about the llvm-commits mailing list