[PATCH] D146770: [Propeller] Match debug info filenames from profiles to distinguish internal linkage functions with the same names.

[Han Shen via Phabricator via llvm-commits]

shenhan added a comment.

Thanks for implementing this.



================
Comment at: llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp:129
+      else if (!DIFilenameStr.empty())
+        return invalidProfileError("Unknown string found: \'" + DIFilenameStr +
+                                   "'.");
----------------
nit: backslash can be dropped in "Unknown string found: \'".


================
Comment at: llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp:160-161
+      if (!R.second)
+        return invalidProfileError("Duplicate profile for function: '" +
+                                   Aliases.front() + "'.");
+      FI = R.first;
----------------
Do you think we shall make this more tolerant? Instead of returning a invalidProfileError, just drop the profile for this specific function?


================
Comment at: llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp:175
+  FunctionNameToDIFilename.clear();
+  for (const auto &F : M) {
+    SmallString<128> DIFilename;
----------------
Could you spell out the full type?


================
Comment at: llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp:183
+      if (CU)
+        DIFilename = sys::path::remove_leading_dotslash(CU->getFilename());
+    }
----------------
I'll apply this to create_llvm_prof too.


================
Comment at: llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp:189-190
+  }
+  if (FunctionNameToDIFilename.empty())
+    errs() << "IT IS EMPTY\n";
+  if (auto Err = ReadProfile())
----------------
This mean there are no function definitions in the module? Is it an error? 
(If it is a real error, we can make it more meaningful, like "module does not contain function defintions, etc...")


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146770/new/

https://reviews.llvm.org/D146770