[PATCH] D140457: [BOLT] Process fragment siblings in lite mode, keep lite mode on

Maksim Panchenko via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 18 18:22:28 PST 2023


maksfb added a comment.

Thanks for refactoring the code.

What happens to stripped binaries? Should we disable lite mode for them?



================
Comment at: bolt/lib/Core/BinaryContext.cpp:504
+    // Check if BF is a fragment of TargetBF or vice versa.
+    return (BF.isFragment() && BF.isParentFragment(TargetBF)) ||
+           (TargetBF->isFragment() && TargetBF->isParentFragment(&BF));
----------------
Do we have to check `isFragment()` before checking `isParentFragment()`? BTW, what do you think about renaming the latter to `isChildOf()`?


================
Comment at: bolt/lib/Rewrite/RewriteInstance.cpp:737
 
+  registerFragments();
+
----------------
Add the call to the end of `discoverFileObjects()`. That's where we register functions, data objects, etc.


================
Comment at: bolt/lib/Rewrite/RewriteInstance.cpp:1299
+    for (StringRef Name : Function.getNames()) {
+      size_t ColdSuffixPos = Name.find(".cold");
+      if (ColdSuffixPos == StringRef::npos)
----------------



================
Comment at: bolt/lib/Rewrite/RewriteInstance.cpp:1303
+      StringRef ParentName = Name.substr(0, ColdSuffixPos);
+      const BinaryData *BD = BC->getBinaryDataByName(ParentName);
+      if (!BD) {
----------------
What if you have global `foo`+`foo.cold.1` and local `foo/1`+`foo.cold.1/1`? What will `foo.cold.1/1` match to?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140457



More information about the llvm-commits mailing list