Skip to content
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

[BOLT][NFC] Extract a function for dump MCInst #67225

Merged
merged 1 commit into from
Nov 21, 2023
Merged

Conversation

llongint
Copy link
Contributor

In GDB debugging, obtaining the assembly representation of MCInst is more intuitive.

@llongint llongint added the BOLT label Sep 23, 2023
@llongint
Copy link
Contributor Author

llongint commented Sep 23, 2023

image

In GDB debugging, sometimes I want to obtain the assembly representation of an MCInst, as it would be more intuitive. However, it seems that such an interface is currently lacking, So I thought maybe we should add one.

@@ -389,6 +389,13 @@ bool BinaryFunction::isForwardCall(const MCSymbol *CalleeSymbol) const {
}
}

void BinaryFunction::dump(const MCInst &Inst) const {
if (!BC.InstPrinter)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me it is more of a BC interace, not BF. Also suggest you to write something on if instprinter is not ready.
Anyway it needs to be used somehow, it could not be just merged without proper usage, tests & etc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, thanks for your suggestion. I will try to modify it to BC interface and add test cases or usage, for testing purposes.

@llongint llongint marked this pull request as draft September 23, 2023 10:51
@llongint llongint marked this pull request as ready for review September 23, 2023 14:52
@llongint llongint changed the title [BOLT] Add a printing function for MCInst [BOLT] Extract a function for dump MCInst Sep 24, 2023
@llongint llongint changed the title [BOLT] Extract a function for dump MCInst [BOLT][NFC] Extract a function for dump MCInst Sep 25, 2023
@llongint
Copy link
Contributor Author

llongint commented Oct 8, 2023

ping.

@llongint
Copy link
Contributor Author

Hi, @aaupov @maksfb @rafaelauler , can I merge this debugging assistance interface?

Copy link
Contributor

@maksfb maksfb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks. Small suggestion: s/dumpInst/dump/ for debugging convenience, but I don't insist.

Extract a function for printing MCInst, so we can call it in GDB to get
a more intuitive assembly representation.
@llongint
Copy link
Contributor Author

LGTM. Thanks. Small suggestion: s/dumpInst/dump/ for debugging convenience, but I don't insist.

Thanks, modified.

@llongint llongint merged commit f3e54f2 into llvm:main Nov 21, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants