[llvm-exegesis] Debug generated disassembly #142540

lakshayk-nv · 2025-06-03T06:01:03Z

Debug generated disassembly by passing argument debug-only="print-gen-assembly" or debug-only=preview-gen-assembly of exegesis call.
--debug-only="print-gen-assembly" debugs the whole generated assembly snippet .
--debug-only=preview-gen-assembly debugs the initial 10 instructions and ending 3 lines.
Thus, We can in glance see the initial setup code like registers setup and instruction followed by truncated middle and finally print out the last 3 instructions.

This helps us look into assembly that exegesis is execution in hardware, Thus, it is simply functionally alias to separate objdump command on the dumped object file.

Just a good to have change for dev experience. :)

Please review: @sjoerdmeijer, @boomanaiden154, @davemgreen

llvmbot · 2025-06-03T06:01:36Z

@llvm/pr-subscribers-tools-llvm-exegesis

Author: Lakshay Kumar (lakshayk-nv)

Changes

Debug generated disassembly by passing argument debug-only="exegesis-benchmark-runner" of exegesis call. Calling /usr/bin/objdump -d on generated object file for debugging generated disassembly.
Just a good to have change for dev experience. :)

Full diff: https://github.com/llvm/llvm-project/pull/142540.diff

1 Files Affected:

(modified) llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp (+34)

diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
index a7771b99e97b1..7c2a6c966106a 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
@@ -20,6 +20,7 @@
 #include "llvm/ADT/Twine.h"
 #include "llvm/Config/llvm-config.h" // for LLVM_ON_UNIX
 #include "llvm/Support/CrashRecoveryContext.h"
+#include "llvm/Support/Debug.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -29,6 +30,7 @@
 #include <cmath>
 #include <memory>
 #include <string>
+#define DEBUG_TYPE "exegesis-benchmark-runner"
 
 #ifdef __linux__
 #ifdef HAVE_LIBPFM
@@ -709,6 +711,38 @@ std::pair<Error, Benchmark> BenchmarkRunner::runConfiguration(
     }
     outs() << "Check generated assembly with: /usr/bin/objdump -d "
            << *ObjectFilePath << "\n";
+
+#ifdef __linux__
+    int StdOutFD, StdErrFD;
+    SmallString<128> StdOutFile, StdErrFile;
+    sys::fs::createTemporaryFile("temp-objdump-out", "txt", StdOutFD,
+                                 StdOutFile);
+    sys::fs::createTemporaryFile("temp-objdump-err", "txt", StdErrFD,
+                                 StdErrFile);
+    std::vector<std::optional<StringRef>> Redirects = {
+        std::nullopt,          // stdin
+        StringRef(StdOutFile), // stdout
+        StringRef(StdErrFile)  // stderr
+    };
+
+    std::string ErrMsg;
+    int Result = sys::ExecuteAndWait(
+        "/usr/bin/objdump", {"/usr/bin/objdump", "-d", *ObjectFilePath},
+        std::nullopt, Redirects, 0, 0, &ErrMsg);
+    auto StdOutBuf = MemoryBuffer::getFile(StdOutFile);
+    if (StdOutBuf && !(*StdOutBuf)->getBuffer().empty())
+      LLVM_DEBUG(dbgs() << "[llvm-exegesis][objdump] Generated assembly:\n"
+                        << (*StdOutBuf)->getBuffer() << '\n');
+    auto StdErrBuf = MemoryBuffer::getFile(StdErrFile);
+    if (StdErrBuf && !(*StdErrBuf)->getBuffer().empty())
+      LLVM_DEBUG(dbgs() << "[llvm-exegesis][objdump] stderr:\n"
+                        << (*StdErrBuf)->getBuffer() << '\n');
+    if (!ErrMsg.empty())
+      LLVM_DEBUG(dbgs() << "[llvm-exegesis][objdump] process error: " << ErrMsg
+                        << '\n');
+    sys::fs::remove(StdOutFile);
+    sys::fs::remove(StdErrFile);
+#endif
   }
 
   if (BenchmarkPhaseSelector < BenchmarkPhaseSelectorE::Measure) {

boomanaiden154

I don't exactly see the point of this. Running objdump (or llvm-objdump/whatever version) on the output of -dump-object-to-disk is intended to cover this exact use case. I don't think the slight convenience of running it automatically by passing -debug-only or -debug outweights the extra code complexity and other problems that will come with this.

It's hard to test because now you're relying on /usr/bin/objdump existing, you have no idea what /usr/bin/objdump actually does or if it even exists, and creating temporary files to invoke an external command is the wrong way to go about this. If we want to print instructions we can spin up a MCInstPrinter to print the vector of MCInsts that we have earlier. That case is still covered by -dump-object-to-disk though.

sjoerdmeijer · 2025-06-09T09:58:16Z

I don't exactly see the point of this. Running objdump (or llvm-objdump/whatever version) on the output of -dump-object-to-disk is intended to cover this exact use case. I don't think the slight convenience of running it automatically by passing -debug-only or -debug outweights the extra code complexity and other problems that will come with this.

This would be a very welcome debug feature for me. I agree that this can be achieved in the way you described it, but it would be so much more efficient for me if I can flip a switch and don't need to do this all manually. Analysing the snippets is really important, i.e. analysing them to see if they truly test what they want to test is important and time consuming, and anything that helps is welcome.

I agree with the trade-off in general, a debug feature shouldn't lead to massive churn in the code, or pose maintainance problems now or later, but this really doesn't seem to be the case here. I.e., it is self-contained, and not a lot of code, partly because it already relies on the infrastructure that is mostly there already.

Long story short, I fully support the change, but I would like to see 2 changes:

I agree we don't want hard-code paths, so we want to remove /usr/bin/objdump, and we want to change that to llvm-objdump. That is already used in some test cases, so that is not a new dependency,
and we need a test-case, because we need one, and also so that we can see how this looks like.

sjoerdmeijer · 2025-06-09T10:04:51Z

I had actually not looked at the implementation very carefully, but now looking and thinking about it, I think I would like to have this available under and option, so that I can toggle this on or off. For example, add an option -print-snippet-assembly or something along those lines.

In terms of implementation, this could be a wrapper around "-dump-object-to-disk" and then disassembling and printing it to stdout, or the current implementation.

Does this (and my previous comment) help with your concerns, @boomanaiden154 ?

boomanaiden154 · 2025-06-09T17:21:08Z

This would be a very welcome debug feature for me. I agree that this can be achieved in the way you described it, but it would be so much more efficient for me if I can flip a switch and don't need to do this all manually. Analysing the snippets is really important, i.e. analysing them to see if they truly test what they want to test is important and time consuming, and anything that helps is welcome.

The snippet (not repeated) is already shown in the benchmark key:

key:
  instructions:
    - 'CMP64rr RSI RSI'
    - 'ADC64ri32_ND RSI RDI i_0x1'

I don't see how this patch will provide significant benefit over that, other than maybe printing everything in a more familiar format.

I agree we don't want hard-code paths, so we want to remove /usr/bin/objdump, and we want to change that to llvm-objdump. That is already used in some test cases, so that is not a new dependency,

I don't see how this is preferrable to a MCInstPrinter.

…xternal dependency outside llvm project

…vm-project into llvm-exegesis-disassembly

github-actions · 2025-07-01T04:02:54Z

✅ With the latest revision this PR passed the C/C++ code formatter.

sjoerdmeijer · 2025-07-01T08:54:40Z

I was just looking at this, and noticed the update, and was wondering why all the pointer authentication stuff is included here. I am assuming that was included by mistake?

lakshayk-nv · 2025-07-01T09:05:54Z

I was just looking at this, and noticed the update, and was wondering why all the pointer authentication stuff is included here. I am assuming that was included by mistake?

Apologies, I messed up this branch. I just wanted to add the testcase commit.
PS: Reverted back those bad commits. Please review the changes and test cases, Thanks.

sjoerdmeijer · 2025-07-01T10:38:45Z

llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp

+      };
+
+      // Preview generated assembly snippet
+      {


How does this work now? Does this mean the snippet is printed twice if option preview-gen-assembly is provided and it is a debug build, is that right?

And --debug-only="preview-gen-assembly" only works with a debug build?

I would prefer to just flip a switch and option, similar to --dump-object-to-disk, so that this works in any build.

How to use:
--debug-only="print-gen-assembly" debugs the whole generated assembly snippet .
--debug-only=preview-gen-assembly debugs the initial 10 instructions and ending 3 lines.
(Updated PR Description accordingly)

No, Given argument of --debug-only="print-gen-assembly" exegesis will print out the generated assembly only once with truncated middle part, so not to fill up whole terminal for large value of min-instructions

$ build/bin/llvm-exegesis -mcpu=neoverse-v2 --dump-object-to-disk=%d --benchmark-phase=measure --min-instructions=10000 --mode=latency --opcode-name=ADCXr --debug-only="preview-gen-assembly" setRegTo is not implemented, results will be unreliable setRegTo is not implemented, results will be unreliable Generated assembly snippet: ''' 0: f81f0ff7 str x23, [sp, #-16]! 4: d2800017 mov x23, #0 8: d280000d mov x13, #0 c: 9a0d02ed adc x13, x23, x13 10: 9a0d02ed adc x13, x23, x13 14: 9a0d02ed adc x13, x23, x13 18: 9a0d02ed adc x13, x23, x13 1c: 9a0d02ed adc x13, x23, x13 20: 9a0d02ed adc x13, x23, x13 24: 9a0d02ed adc x13, x23, x13 ... (9992 more instructions) 9c48: 9a0d02ed adc x13, x23, x13 9c4c: f84107f7 ldr x23, [sp], #16 9c50: d65f03c0 ret ''' Check generated assembly with: /usr/bin/objdump -d %d --- mode: latency key: instructions: - 'ADCXr X13 X23 X13' config: '' register_initial_values: - 'X23=0x0' - 'X13=0x0' - 'NZCV=0x0' cpu_name: neoverse-v2 llvm_triple: aarch64-unknown-linux-gnu min_instructions: 10000 measurements: - { key: latency, value: 1.0036, per_snippet_value: 1.0036, validation_counters: {} } error: '' info: Repeating a single explicitly serial instruction assembled_snippet: F70F1FF8170080D20D0080D2ED020D9AED020D9AED020D9AED020D9AF70741F8C0035FD6 ...

If we explicitly give argument --debug-only="preview-gen-assembly,print-gen-assembly" to exegesis, both preview and print debug will result in duplicate print.

And --debug-only="preview-gen-assembly" only works with a debug build?

No, I checked this work for both Debug and Release build (UPDATE:with assertion on).

I would prefer to just flip a switch and option, similar to --dump-object-to-disk, so that this works in any build.

Given that, Can you reconsider this (additional argument) because we will require two arguments if not for --debug-only one for whole assembly print and another for preview.

(Currently, Given the argument of --dump-object-to-disk=<filename> exegesis just stdout
Check generated assembly with: /usr/bin/objdump -d %d. This object file is to be objdump in next command outside exegesis)

In terms of implementation, this could be a wrapper around "-dump-object-to-disk"

I know you previously also asked for this. If this seems necessary, can you please expand on wrapper thing.

Thanks for the clarifications.

What I have suggested could have been an alternative way to implement this, but I am happy with the implementation here as it is nice and concise and doesn't depend on external tools. So, ignore that suggestion.

boomanaiden154

Thanks for the update, this looks a lot better.

llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp

boomanaiden154 · 2025-07-01T22:09:58Z

llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp

+    // Extract the actual function bytes from the object file
+    std::vector<uint8_t> FunctionBytes;
+    if (auto Err = getBenchmarkFunctionBytes(*Snippet, FunctionBytes)) {
+      dbgs() << "Failed to extract function bytes: " << toString(std::move(Err))


Why the preference for a soft failure here?

Printing generating assembly as we agreed, quite an optional feature, orthogonal to getting hardware measurement. Thus, didn't want to interfere with default functioning of exegesis and simple print out any error/warning in this flow and continue with soft failure.

Reverting previous reasoning.
Introduced ExitWithError(...) instead of soft failure, because i guess if there is problem in generated snippet we can figure it out here rather than at execution of snippet.

llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp

sjoerdmeijer · 2025-07-02T12:39:38Z

llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp

+  dbgs() << "```\n";
+  size_t N = Instructions.size();
+  // Print first "PreviewFirst" lines or all if less
+  for (size_t i = 0; i < std::min(size_t(PreviewFirst), N); ++i) {


Please drop the curly brackets { and } for any statements (if statements or for-loops) that have only 1 statement in this. This is much easier to read, and should be documented in the LLVM coding guidelines.

Resolved, Thanks!

sjoerdmeijer · 2025-07-02T12:46:18Z

llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp

+  }
+  if (N > (PreviewFirst + PreviewLast)) {
+    if (Preview) {
+      dbgs() << "...\t(" << (N - PreviewFirst - PreviewLast)


So yeah, debug vs. release builds had me confused earlier. I guess this debug stream is available even in non-Debug mode, but does it make sense to use outs() instead? I see that being used elsewhere, and guess that is the slightly more appropriate output stream for this?

llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp

…neration

sjoerdmeijer

I like the output and think this will be very useful to me. Ideally I would like to have this available in a release build without assertions, but let's get some experience first before we promote this to an option, so LGTM.

I've left two nits that don't need another review if addressed.

llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp

llvm/test/tools/llvm-exegesis/AArch64/debug-gen-asm.s

boomanaiden154 · 2025-07-03T23:15:55Z

Ideally I would like to have this available in a release build without assertions, but let's get some experience first before we promote this to an option, so LGTM.

Why the preference for enabling this without assertions? This seems almost exclusively a debug feature. llvm-exegesis is also pretty much exclusively an LLVM development tool, so there's not really a need to ship a release build anywhere. At least that's how I see it, not sure if others use it much differently.

llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp

llvm/test/tools/llvm-exegesis/AArch64/debug-gen-asm.s

…s to print

sjoerdmeijer · 2025-07-04T10:47:33Z

Sorry for the confusion and putting noise on the line, but @boomanaiden154 was happy with this as a debug feature. Let's go with that. Hopefully it's not too much work to restore the LLVM_DEBUG and dbgs() things.

lakshayk-nv added 2 commits May 30, 2025 03:46

[llvm-exegesis] Added Debug flag for print disassembly using objdump

3c9421c

[llvm-exegesis] Formatting changes

7d61a14

llvmbot added the tools:llvm-exegesis label Jun 3, 2025

RKSimon requested a review from boomanaiden154 June 3, 2025 07:00

boomanaiden154 reviewed Jun 3, 2025

View reviewed changes

Merge branch 'main' into llvm-exegesis-disassembly

30b5740

sjoerdmeijer requested review from sjoerdmeijer and davemgreen June 9, 2025 09:58

lakshayk-nv added 2 commits June 30, 2025 20:57

[llvm-exegesis] Debug (print or preview) generated assembly without e…

e53e3c8

…xternal dependency outside llvm project

Merge branch 'llvm-exegesis-disassembly' of github.com:lakshayk-nv/ll…

a194ac0

…vm-project into llvm-exegesis-disassembly

lakshayk-nv added 2 commits June 30, 2025 21:07

[llvm-exegesis] Clang format

8caf041

[llvm-exegesis] Cleanup changes

b57c3ab

lakshayk-nv requested a review from boomanaiden154 July 1, 2025 08:06

lakshayk-nv force-pushed the llvm-exegesis-disassembly branch from a8160ae to b57c3ab Compare July 1, 2025 09:28

[llvm-exegesis] Add tests for generated assembly snippets in AArch64

7e9550e

sjoerdmeijer reviewed Jul 1, 2025

View reviewed changes

boomanaiden154 reviewed Jul 1, 2025

View reviewed changes

[llvm-exegesis] Refactor assembly snippet printing into helper functions

1e97fad

sjoerdmeijer reviewed Jul 2, 2025

View reviewed changes

llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp Outdated Show resolved Hide resolved

sjoerdmeijer reviewed Jul 2, 2025

View reviewed changes

llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp Outdated Show resolved Hide resolved

llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp Outdated Show resolved Hide resolved

llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp Outdated Show resolved Hide resolved

[llvm-exegesis] Refactor instruction decoding and assembly snippet ge…

989d7b0

…neration

sjoerdmeijer approved these changes Jul 3, 2025

View reviewed changes

llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp Show resolved Hide resolved

llvm/test/tools/llvm-exegesis/AArch64/debug-gen-asm.s Outdated Show resolved Hide resolved

boomanaiden154 reviewed Jul 3, 2025

View reviewed changes

lakshayk-nv added 2 commits July 4, 2025 01:06

[llvm-exegesis] Introduced --print-gen-assembly=<> for number of line…

a987b50

…s to print

[llvm-exegesis] Newline added in testfile

4acfe17

[llvm-exegesis] Debug generated disassembly #142540

Are you sure you want to change the base?

[llvm-exegesis] Debug generated disassembly #142540

Uh oh!

Conversation

lakshayk-nv commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jun 3, 2025

Uh oh!

boomanaiden154 left a comment

Choose a reason for hiding this comment

Uh oh!

sjoerdmeijer commented Jun 9, 2025

Uh oh!

sjoerdmeijer commented Jun 9, 2025

Uh oh!

boomanaiden154 commented Jun 9, 2025

Uh oh!

github-actions bot commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sjoerdmeijer commented Jul 1, 2025

Uh oh!

lakshayk-nv commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lakshayk-nv Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lakshayk-nv Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boomanaiden154 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sjoerdmeijer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

boomanaiden154 commented Jul 3, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sjoerdmeijer commented Jul 4, 2025

Uh oh!

Uh oh!

lakshayk-nv commented Jun 3, 2025 •

edited

Loading

github-actions bot commented Jul 1, 2025 •

edited

Loading

lakshayk-nv commented Jul 1, 2025 •

edited

Loading

lakshayk-nv Jul 1, 2025 •

edited

Loading

lakshayk-nv Jul 1, 2025 •

edited

Loading