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

[GOFF] Wrap debug output with LLVM_DEBUG #87252

Merged
merged 1 commit into from
Apr 1, 2024

Conversation

redstar
Copy link
Member

@redstar redstar commented Apr 1, 2024

The content of a GOFF record is always dumped if NDEBUG is not defined,
which produces rather confusing output. This changes wrap the dumping
code in LLVM_DEBUG, so the dump is only done when debug output of this
module is requested.

The content of a GOFF record is always dumped if NDEBUG is not defined,
which produces rather confusing output. This changes wrap the dumping
code in LLVM_DEBUG, so the dump is only done when debug output of this
module is requested.
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 1, 2024

@llvm/pr-subscribers-llvm-binary-utilities

Author: Kai Nacke (redstar)

Changes

The content of a GOFF record is always dumped if NDEBUG is not defined,
which produces rather confusing output. This changes wrap the dumping
code in LLVM_DEBUG, so the dump is only done when debug output of this
module is requested.


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

1 Files Affected:

  • (modified) llvm/lib/Object/GOFFObjectFile.cpp (+3-6)
diff --git a/llvm/lib/Object/GOFFObjectFile.cpp b/llvm/lib/Object/GOFFObjectFile.cpp
index 76a13559ebfe35..d3dfd5d1540cfe 100644
--- a/llvm/lib/Object/GOFFObjectFile.cpp
+++ b/llvm/lib/Object/GOFFObjectFile.cpp
@@ -104,16 +104,13 @@ GOFFObjectFile::GOFFObjectFile(MemoryBufferRef Object, Error &Err)
       PrevContinuationBits = I[1] & 0x03;
       continue;
     }
-
-#ifndef NDEBUG
-    for (size_t J = 0; J < GOFF::RecordLength; ++J) {
+    LLVM_DEBUG(for (size_t J = 0; J < GOFF::RecordLength; ++J) {
       const uint8_t *P = I + J;
       if (J % 8 == 0)
         dbgs() << "  ";
-
       dbgs() << format("%02hhX", *P);
-    }
-#endif
+    });
+
     switch (RecordType) {
     case GOFF::RT_ESD: {
       // Save ESD record.

@redstar redstar self-assigned this Apr 1, 2024
Copy link
Contributor

@perry-ca perry-ca left a comment

Choose a reason for hiding this comment

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

looks good. It might be helpful to but a tag at the beginning of the output so people know this is the GOFF record and a new line at the end.

Copy link
Contributor

@abhina-sree abhina-sree left a comment

Choose a reason for hiding this comment

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

LGTM

@redstar
Copy link
Member Author

redstar commented Apr 1, 2024

looks good. It might be helpful to but a tag at the beginning of the output so people know this is the GOFF record and a new line at the end.

I agree that the formatting of the output could be better. (The user must request --debug-only=goff so there is no need to tag the output.) I leave this to a later change, since my main point is to remove the debug output in normal operation.

@redstar redstar merged commit 6634c3e into llvm:main Apr 1, 2024
6 checks passed
@redstar redstar deleted the knacke/wrapdebugoutput branch April 16, 2024 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants