[PATCH] D89712: [CSSPGO][llvm-profgen] Disassemble text sections

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.cpp:114
+                                        SectionSymbolsTy &Symbols,
+                                        uint64_t ImageLoadAddr,
+                                        uint64_t SectionRVA, uint64_t SectSize,
----------------
No need to pass this in as a parameter. Can just use the member field `PreferredBaseAddress` instead.


================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.cpp:116
+                                        uint64_t SectionRVA, uint64_t SectSize,
+                                        StringRef &FileName) {
+
----------------
StringRef type doesn't need to be passed by reference as a convention.


================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.cpp:116
+                                        uint64_t SectionRVA, uint64_t SectSize,
+                                        StringRef &FileName) {
+
----------------
hoy wrote:
> StringRef type doesn't need to be passed by reference as a convention.
The function can return false upon a disassemble error and let the caller handle the error so that `FileName` is not needed here. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89712