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

Display symbol names "compile -D" #8637

Merged
merged 1 commit into from
May 21, 2024

Conversation

lpereira
Copy link
Contributor

@lpereira lpereira commented May 16, 2024

Another one in the series to improve the debuggability quality of life: display the symbol name in the "compile -D" command.

No tests seems to expect anything with the string "Disassembly of {num} bytes", so no tests have been updated.

Here's an output sample (excerpt from compile -D cranelift/filetests/filetests/wasm/f64-arith.clif):

Disassembly of 44 bytes <%f64_copysign>:
   0:	55                   	pushq	%rbp
   1:	48 89 e5             	movq	%rsp, %rbp
   4:	48 b9 00 00 00 00 00 00 00 80	
				movabsq	$9223372036854775808, %rcx
   e:	66 48 0f 6e f9       	movq	%rcx, %xmm7
  13:	66 0f 6f d0          	movdqa	%xmm0, %xmm2
  17:	66 0f 6f c7          	movdqa	%xmm7, %xmm0
  1b:	66 0f 55 c2          	andnpd	%xmm2, %xmm0
  1f:	66 0f 54 f9          	andpd	%xmm1, %xmm7
  23:	66 0f 56 c7          	orpd	%xmm7, %xmm0
  27:	48 89 ec             	movq	%rbp, %rsp
  2a:	5d                   	popq	%rbp
  2b:	c3                   	retq		

@lpereira lpereira requested a review from a team as a code owner May 16, 2024 17:45
@lpereira lpereira requested review from elliottt and removed request for a team May 16, 2024 17:45
@lpereira lpereira force-pushed the disasm-with-symbol-names branch 2 times, most recently from 9a25c30 to c8e852e Compare May 16, 2024 17:53
@github-actions github-actions bot added the cranelift Issues related to the Cranelift code generator label May 16, 2024
@elliottt
Copy link
Member

Adding the name seems like a really nice improvement, but I'm not sure about the additional hex and byte dumps, given that Capstone is already printing out the hex bytes of the instructions. Was there a specific case that you had in mind where they were improving debugging?

This aids debugging slightly, as we now don't have to go through each CLIF
source and the disassembly output to understand whichi function is which.
@lpereira lpereira changed the title Display symbol names and a hex dump in "compile -D" Display symbol names "compile -D" May 21, 2024
@lpereira
Copy link
Contributor Author

Rebased and force-pushed with only the change to show the function name.

Copy link
Member

@elliottt elliottt left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@elliottt elliottt added this pull request to the merge queue May 21, 2024
Merged via the queue into bytecodealliance:main with commit 55909a3 May 21, 2024
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift Issues related to the Cranelift code generator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants