[PATCH] D126177: [BOLT] [AArch64] Handle data markers spanning multiple functions

Denis via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 25 02:17:30 PDT 2022


treapster updated this revision to Diff 431908.
treapster added a comment.

Thank you for the feedback, @rafaelauler ! I fixed the test and removed packed attribute as you said.

As for the markerType-related functions, before this patch they depended on BC to check whether arch is AArch64, so I preserved this check and moved them to BC itself. However, I don't think this check is essential - because theoretically nothing stops you from putting data in text on other architectures and marking it with the same $x/$d symbols. If we agree on it, we can remove this dependency and make getMarkerType and related functions either methods of SymbolRef or freestanding functions that only get passed symbolRef.

Regarding comparison function, I think it would be weird for a binary to have more than one marker at the same offset in the first place, but even if it had, they just would be considered equal by the comparison function - same as with two functions on the same offset or anything else. I don't see where it breaks requirements of sort. The basic requirement is that comparator cannot return true both ways, and mine does not.

And speaking of constantIsland map, I don't think marking every island will impact performance or memory usage in any significant way, but as a result data in text will be handled properly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126177

Files:
  bolt/include/bolt/Core/BinaryContext.h
  bolt/include/bolt/Core/BinaryFunction.h
  bolt/lib/Core/BinaryContext.cpp
  bolt/lib/Core/BinaryFunction.cpp
  bolt/lib/Rewrite/RewriteInstance.cpp
  bolt/test/AArch64/unmarked-data.s

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D126177.431908.patch
Type: text/x-patch
Size: 13130 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220525/87fe8f7c/attachment.bin>


More information about the llvm-commits mailing list