-
Notifications
You must be signed in to change notification settings - Fork 15.4k
release/21.x: [Hexagon][llvm-objdump] Start a fresh packet at symbol boundaries. (#163466) #163662
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@androm3da What do you think about merging this PR to the release branch? |
|
@llvm/pr-subscribers-llvm-binary-utilities @llvm/pr-subscribers-backend-hexagon Author: None (llvmbot) ChangesBackport 0cdebda Requested by: @quic-areg Full diff: https://github.com/llvm/llvm-project/pull/163662.diff 3 Files Affected:
diff --git a/llvm/lib/Target/Hexagon/Disassembler/HexagonDisassembler.cpp b/llvm/lib/Target/Hexagon/Disassembler/HexagonDisassembler.cpp
index bcddb540d35dc..c48cf5e6353ac 100644
--- a/llvm/lib/Target/Hexagon/Disassembler/HexagonDisassembler.cpp
+++ b/llvm/lib/Target/Hexagon/Disassembler/HexagonDisassembler.cpp
@@ -64,6 +64,10 @@ class HexagonDisassembler : public MCDisassembler {
void remapInstruction(MCInst &Instr) const;
+ Expected<bool> onSymbolStart(SymbolInfoTy &Symbol, uint64_t &Size,
+ ArrayRef<uint8_t> Bytes,
+ uint64_t Address) const override;
+
private:
bool makeBundle(ArrayRef<uint8_t> Bytes, uint64_t Address,
uint64_t &BytesToSkip, raw_ostream &CS) const;
@@ -604,6 +608,18 @@ DecodeStatus HexagonDisassembler::getSingleInstruction(MCInst &MI, MCInst &MCB,
return Result;
}
+Expected<bool> HexagonDisassembler::onSymbolStart(SymbolInfoTy &Symbol,
+ uint64_t &Size,
+ ArrayRef<uint8_t> Bytes,
+ uint64_t Address) const {
+ // At the start of a symbol, force a fresh packet by resetting any
+ // in-progress bundle state. This prevents packets from straddling label
+ // boundaries when data (e.g. jump tables) appears in between.
+ Size = 0;
+ resetBundle();
+ return true;
+}
+
static DecodeStatus DecodeRegisterClass(MCInst &Inst, unsigned RegNo,
ArrayRef<MCPhysReg> Table) {
if (RegNo < Table.size()) {
diff --git a/llvm/test/tools/llvm-objdump/ELF/Hexagon/packet-reset-on-label.s b/llvm/test/tools/llvm-objdump/ELF/Hexagon/packet-reset-on-label.s
new file mode 100644
index 0000000000000..02a52bbb3fbd8
--- /dev/null
+++ b/llvm/test/tools/llvm-objdump/ELF/Hexagon/packet-reset-on-label.s
@@ -0,0 +1,23 @@
+// RUN: llvm-mc -triple=hexagon -mcpu=hexagonv75 -filetype=obj %s \
+// RUN: | llvm-objdump -d - \
+// RUN: | FileCheck %s
+
+foo:
+ { nop }
+ /// a nop without end-of-packet bits set to simulate data that is
+ /// not a proper packet end.
+ .long 0x7f004000
+bar:
+ { nop
+ nop
+ }
+
+// CHECK-LABEL: <foo>:
+// CHECK: { nop }
+// CHECK-NEXT: { nop
+
+/// The instruction starting after <bar> should start in a new packet.
+// CHECK-LABEL: <bar>:
+// CHECK: { nop
+// CHECK-NEXT: nop }
+
diff --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp
index 74eb9033c8e2c..221b884e0c06c 100644
--- a/llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ b/llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -717,11 +717,17 @@ class PrettyPrinter {
} while (!Comments.empty());
FOS.flush();
}
+
+ // Hook invoked when starting to disassemble a symbol at the current position.
+ // Default is no-op.
+ virtual void onSymbolStart() {}
};
PrettyPrinter PrettyPrinterInst;
class HexagonPrettyPrinter : public PrettyPrinter {
public:
+ void onSymbolStart() override { reset(); }
+
void printLead(ArrayRef<uint8_t> Bytes, uint64_t Address,
formatted_raw_ostream &OS) {
if (LeadingAddr)
@@ -2216,6 +2222,8 @@ disassembleObject(ObjectFile &Obj, const ObjectFile &DbgObj,
Start += Size;
break;
}
+ // Allow targets to reset any per-symbol state.
+ DT->Printer->onSymbolStart();
formatted_raw_ostream FOS(OS);
Index = Start;
if (SectionAddr < StartAddress)
|
…lvm#163466) Hexagon packets can visually straddle labels when data (e.g. jump tables) in the text section does not carry end-of-packet bits. In such cases the next instruction, even at a new symbol, appears to continue the previous packet. This patch resets packet state when encountering a new symbol so that packets at symbol starts are guaranteed to start in their own packet. (cherry picked from commit 0cdebda)
|
@quic-areg (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR. |
Backport 0cdebda
Requested by: @quic-areg