<div dir="ltr">Looks like a good cleanup, would you mind documenting the structure and fields as a follow-on?<div><br></div><div>What are your plans here?<br><br>Thanks!<br><br>-eric<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Feb 10, 2020 at 4:23 PM via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
Author: diggerlin<br>
Date: 2020-02-10T19:23:01-05:00<br>
New Revision: 09d26b79d295f1d2efb94c0ebc5db9d0d881454e<br>
<br>
URL: <a href="https://github.com/llvm/llvm-project/commit/09d26b79d295f1d2efb94c0ebc5db9d0d881454e" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/09d26b79d295f1d2efb94c0ebc5db9d0d881454e</a><br>
DIFF: <a href="https://github.com/llvm/llvm-project/commit/09d26b79d295f1d2efb94c0ebc5db9d0d881454e.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/09d26b79d295f1d2efb94c0ebc5db9d0d881454e.diff</a><br>
<br>
LOG: [NFC] Refactor the tuple of symbol information with structure for llvm-objdump<br>
<br>
SUMMARY:<br>
<br>
refator the std::tuple<uint64_t, StringRef, uint8_t> to structor<br>
<br>
Reviewers: daltenty<br>
Subscribers: wuzish, nemanjai, hiraditya<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D74240" rel="noreferrer" target="_blank">https://reviews.llvm.org/D74240</a><br>
<br>
Added: <br>
<br>
<br>
Modified: <br>
llvm/include/llvm/MC/MCDisassembler/MCDisassembler.h<br>
llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp<br>
llvm/tools/llvm-objdump/llvm-objdump.cpp<br>
<br>
Removed: <br>
<br>
<br>
<br>
################################################################################<br>
diff --git a/llvm/include/llvm/MC/MCDisassembler/MCDisassembler.h b/llvm/include/llvm/MC/MCDisassembler/MCDisassembler.h<br>
index 76c5215264bc..05e47deadfc6 100644<br>
--- a/llvm/include/llvm/MC/MCDisassembler/MCDisassembler.h<br>
+++ b/llvm/include/llvm/MC/MCDisassembler/MCDisassembler.h<br>
@@ -9,12 +9,26 @@<br>
#ifndef LLVM_MC_MCDISASSEMBLER_MCDISASSEMBLER_H<br>
#define LLVM_MC_MCDISASSEMBLER_MCDISASSEMBLER_H<br>
<br>
+#include "llvm/ADT/StringRef.h"<br>
#include "llvm/MC/MCDisassembler/MCSymbolizer.h"<br>
#include <cstdint><br>
#include <memory><br>
+#include <vector><br>
<br>
namespace llvm {<br>
<br>
+struct SymbolInfoTy {<br>
+ uint64_t Addr;<br>
+ StringRef Name;<br>
+ uint8_t Type;<br>
+<br>
+ SymbolInfoTy(uint64_t Addr, StringRef Name, uint8_t Type):<br>
+ Addr(Addr),Name(Name),Type(Type) {};<br>
+};<br>
+<br>
+using SectionSymbolsTy = std::vector<SymbolInfoTy>;<br>
+<br>
+<br>
template <typename T> class ArrayRef;<br>
class StringRef;<br>
class MCContext;<br>
<br>
diff --git a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp<br>
index 419513bdc248..8945b576ae78 100644<br>
--- a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp<br>
+++ b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp<br>
@@ -1202,8 +1202,6 @@ bool AMDGPUSymbolizer::tryAddingSymbolicOperand(MCInst &Inst,<br>
raw_ostream &/*cStream*/, int64_t Value,<br>
uint64_t /*Address*/, bool IsBranch,<br>
uint64_t /*Offset*/, uint64_t /*InstSize*/) {<br>
- using SymbolInfoTy = std::tuple<uint64_t, StringRef, uint8_t>;<br>
- using SectionSymbolsTy = std::vector<SymbolInfoTy>;<br>
<br>
if (!IsBranch) {<br>
return false;<br>
@@ -1215,11 +1213,11 @@ bool AMDGPUSymbolizer::tryAddingSymbolicOperand(MCInst &Inst,<br>
<br>
auto Result = std::find_if(Symbols->begin(), Symbols->end(),<br>
[Value](const SymbolInfoTy& Val) {<br>
- return std::get<0>(Val) == static_cast<uint64_t>(Value)<br>
- && std::get<2>(Val) == ELF::STT_NOTYPE;<br>
+ return Val.Addr == static_cast<uint64_t>(Value)<br>
+ && Val.Type == ELF::STT_NOTYPE;<br>
});<br>
if (Result != Symbols->end()) {<br>
- auto *Sym = Ctx.getOrCreateSymbol(std::get<1>(*Result));<br>
+ auto *Sym = Ctx.getOrCreateSymbol(Result->Name);<br>
const auto *Add = MCSymbolRefExpr::create(Sym, Ctx);<br>
Inst.addOperand(MCOperand::createExpr(Add));<br>
return true;<br>
<br>
diff --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp<br>
index dc8a4b705c53..6719aed44912 100644<br>
--- a/llvm/tools/llvm-objdump/llvm-objdump.cpp<br>
+++ b/llvm/tools/llvm-objdump/llvm-objdump.cpp<br>
@@ -341,7 +341,15 @@ static StringSet<> DisasmFuncsSet;<br>
StringSet<> FoundSectionSet;<br>
static StringRef ToolName;<br>
<br>
-typedef std::vector<std::tuple<uint64_t, StringRef, uint8_t>> SectionSymbolsTy;<br>
+static bool operator<(const SymbolInfoTy& P1 ,const SymbolInfoTy& P2) {<br>
+ if (P1.Addr < P2.Addr)<br>
+ return true;<br>
+<br>
+ if (P1.Addr == P2.Addr)<br>
+ return P1.Name < P2.Name;<br>
+<br>
+ return false;<br>
+}<br>
<br>
namespace {<br>
struct FilterResult {<br>
@@ -1229,8 +1237,8 @@ static void disassembleObject(const Target *TheTarget, const ObjectFile *Obj,<br>
std::vector<MappingSymbolPair> MappingSymbols;<br>
if (hasMappingSymbols(Obj)) {<br>
for (const auto &Symb : Symbols) {<br>
- uint64_t Address = std::get<0>(Symb);<br>
- StringRef Name = std::get<1>(Symb);<br>
+ uint64_t Address = Symb.Addr;<br>
+ StringRef Name = Symb.Name;<br>
if (Name.startswith("$d"))<br>
MappingSymbols.emplace_back(Address - SectionAddr, 'd');<br>
if (Name.startswith("$x"))<br>
@@ -1264,10 +1272,10 @@ static void disassembleObject(const Target *TheTarget, const ObjectFile *Obj,<br>
<br>
StringRef SectionName = unwrapOrError(Section.getName(), Obj->getFileName());<br>
// If the section has no symbol at the start, just insert a dummy one.<br>
- if (Symbols.empty() || std::get<0>(Symbols[0]) != 0) {<br>
+ if (Symbols.empty() || Symbols[0].Addr != 0) {<br>
Symbols.insert(<br>
Symbols.begin(),<br>
- std::make_tuple(SectionAddr, SectionName,<br>
+ SymbolInfoTy(SectionAddr, SectionName,<br>
Section.isText() ? ELF::STT_FUNC : ELF::STT_OBJECT));<br>
}<br>
<br>
@@ -1289,7 +1297,7 @@ static void disassembleObject(const Target *TheTarget, const ObjectFile *Obj,<br>
std::vector<RelocationRef>::const_iterator RelEnd = Rels.end();<br>
// Disassemble symbol by symbol.<br>
for (unsigned SI = 0, SE = Symbols.size(); SI != SE; ++SI) {<br>
- std::string SymbolName = std::get<1>(Symbols[SI]).str();<br>
+ std::string SymbolName = Symbols[SI].Name.str();<br>
if (Demangle)<br>
SymbolName = demangle(SymbolName);<br>
<br>
@@ -1298,7 +1306,7 @@ static void disassembleObject(const Target *TheTarget, const ObjectFile *Obj,<br>
if (!DisasmFuncsSet.empty() && !DisasmFuncsSet.count(SymbolName))<br>
continue;<br>
<br>
- uint64_t Start = std::get<0>(Symbols[SI]);<br>
+ uint64_t Start = Symbols[SI].Addr;<br>
if (Start < SectionAddr || StopAddress <= Start)<br>
continue;<br>
else<br>
@@ -1308,7 +1316,7 @@ static void disassembleObject(const Target *TheTarget, const ObjectFile *Obj,<br>
// --stop-address.<br>
uint64_t End = std::min<uint64_t>(SectionAddr + SectSize, StopAddress);<br>
if (SI + 1 < SE)<br>
- End = std::min(End, std::get<0>(Symbols[SI + 1]));<br>
+ End = std::min(End, Symbols[SI + 1].Addr);<br>
if (Start >= End || End <= StartAddress)<br>
continue;<br>
Start -= SectionAddr;<br>
@@ -1323,12 +1331,12 @@ static void disassembleObject(const Target *TheTarget, const ObjectFile *Obj,<br>
}<br>
<br>
if (Obj->isELF() && Obj->getArch() == Triple::amdgcn) {<br>
- if (std::get<2>(Symbols[SI]) == ELF::STT_AMDGPU_HSA_KERNEL) {<br>
+ if (Symbols[SI].Type == ELF::STT_AMDGPU_HSA_KERNEL) {<br>
// skip amd_kernel_code_t at the begining of kernel symbol (256 bytes)<br>
Start += 256;<br>
}<br>
if (SI == SE - 1 ||<br>
- std::get<2>(Symbols[SI + 1]) == ELF::STT_AMDGPU_HSA_KERNEL) {<br>
+ Symbols[SI + 1].Type == ELF::STT_AMDGPU_HSA_KERNEL) {<br>
// cut trailing zeroes at the end of kernel<br>
// cut up to 256 bytes<br>
const uint64_t EndAlign = 256;<br>
@@ -1367,7 +1375,7 @@ static void disassembleObject(const Target *TheTarget, const ObjectFile *Obj,<br>
// only disassembling text (applicable all architectures), we are in a<br>
// situation where we must print the data and not disassemble it.<br>
if (Obj->isELF() && !DisassembleAll && Section.isText()) {<br>
- uint8_t SymTy = std::get<2>(Symbols[SI]);<br>
+ uint8_t SymTy = Symbols[SI].Type;<br>
if (SymTy == ELF::STT_OBJECT || SymTy == ELF::STT_COMMON) {<br>
dumpELFData(SectionAddr, Index, End, Bytes);<br>
Index = End;<br>
@@ -1375,7 +1383,7 @@ static void disassembleObject(const Target *TheTarget, const ObjectFile *Obj,<br>
}<br>
<br>
bool CheckARMELFData = hasMappingSymbols(Obj) &&<br>
- std::get<2>(Symbols[SI]) != ELF::STT_OBJECT &&<br>
+ Symbols[SI].Type != ELF::STT_OBJECT &&<br>
!DisassembleAll;<br>
while (Index < End) {<br>
// ARM and AArch64 ELF binaries can interleave data and text in the<br>
@@ -1472,21 +1480,21 @@ static void disassembleObject(const Target *TheTarget, const ObjectFile *Obj,<br>
// the target, find the nearest preceding absolute symbol.<br>
auto TargetSym = partition_point(<br>
*TargetSectionSymbols,<br>
- [=](const std::tuple<uint64_t, StringRef, uint8_t> &O) {<br>
- return std::get<0>(O) <= Target;<br>
+ [=](const SymbolInfoTy &O) {<br>
+ return O.Addr <= Target;<br>
});<br>
if (TargetSym == TargetSectionSymbols->begin()) {<br>
TargetSectionSymbols = &AbsoluteSymbols;<br>
TargetSym = partition_point(<br>
AbsoluteSymbols,<br>
- [=](const std::tuple<uint64_t, StringRef, uint8_t> &O) {<br>
- return std::get<0>(O) <= Target;<br>
+ [=](const SymbolInfoTy &O) {<br>
+ return O.Addr <= Target;<br>
});<br>
}<br>
if (TargetSym != TargetSectionSymbols->begin()) {<br>
--TargetSym;<br>
- uint64_t TargetAddress = std::get<0>(*TargetSym);<br>
- StringRef TargetName = std::get<1>(*TargetSym);<br>
+ uint64_t TargetAddress = TargetSym->Addr;<br>
+ StringRef TargetName = TargetSym->Name;<br>
outs() << " <" << TargetName;<br>
uint64_t Disp = Target - TargetAddress;<br>
if (Disp)<br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>