Skip to content

Conversation

@andrurogerz
Copy link
Contributor

@andrurogerz andrurogerz commented May 19, 2025

Purpose

Reverts the preprocessor guards added to the dump() methods in llvm/Support/ScaledNumber.h in #139938 so that the header can be included when building an external project in debug mode against a release LLVM build.

Overview

This is a clean revert of two files modified in #139938. The rest of that change should not cause similar problems.

Background

The following new build error was reported on #139938, which was merged last week:

module.cpp:(.text._ZNK4llvm12ScaledNumberImE4dumpEv[_ZNK4llvm12ScaledNumberImE4dumpEv]+0x34): undefined reference to `llvm::ScaledNumberBase::dump(unsigned long, short, int)'

See further discussion on #139938.

Validation

Implemented a simple external LLVM project to reproduce the issue.
Verified the the following link failure is observed on LLVM main (Windows + Clang) without this change:

C:\WINDOWS\system32\cmd.exe /C "cd . && C:\PROGRA~1\LLVM\bin\CLANG_~1.EXE -nostartfiles -nostdlib -O0 -g -Xclang -gcodeview -D_DEBUG -D_DLL -D_MT -Xclang --dependent-lib=msvcrtd -Xlinker /subsystem:console  -fuse-ld=lld-link CMakeFiles/llvm-dump-test.dir/main.cxx.obj -o llvm-dump-test.exe -Xlinker /MANIFEST:EMBED -Xlinker /implib:llvm-dump-test.lib -Xlinker /pdb:llvm-dump-test.pdb -Xlinker /version:0.0   -lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 -loldnames  && cd ."
lld-link: error: undefined symbol: public: static void __cdecl llvm::ScaledNumberBase::dump(unsigned __int64, short, int)
>>> referenced by S:\llvm\llvm-project\llvm\include\llvm\Support\ScaledNumber.h:614
>>>               CMakeFiles/llvm-dump-test.dir/main.cxx.obj:(public: void __cdecl llvm::ScaledNumber<unsigned __int64>::dump(void) const)
clang++: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.

Verified the link issue is resolved after applying this change.

Also, manually included all header files that were modified in #139938 in the test program and verified there are no other link errors.

@llvmbot
Copy link
Member

llvmbot commented May 19, 2025

@llvm/pr-subscribers-llvm-support

Author: Andrew Rogers (andrurogerz)

Changes

Purpose

Reverts the preprocessor guards added to the dump() methods in llvm/Support/ScaledNumber.h in #139938 so that the header can be included when building an external project in debug mode against a release LLVM build.

Overview

This is a clean revert of two files modified in #139938. The rest of that change should not cause similar problems.

Background

The following new build error was reported on #139938, which was merged last week:

module.cpp:(.text._ZNK4llvm12ScaledNumberImE4dumpEv[_ZNK4llvm12ScaledNumberImE4dumpEv]+0x34): undefined reference to `llvm::ScaledNumberBase::dump(unsigned long, short, int)'

See further discussion on #139938.

Validation

Implemented a simple external LLVM project to reproduce the issue.
Verified the the following link failure is observed on LLVM main (Windows + Clang) without this change:

C:\WINDOWS\system32\cmd.exe /C "cd . &amp;&amp; C:\PROGRA~1\LLVM\bin\CLANG_~1.EXE -nostartfiles -nostdlib -O0 -g -Xclang -gcodeview -D_DEBUG -D_DLL -D_MT -Xclang --dependent-lib=msvcrtd -Xlinker /subsystem:console  -fuse-ld=lld-link CMakeFiles/llvm-dump-test.dir/main.cxx.obj -o llvm-dump-test.exe -Xlinker /MANIFEST:EMBED -Xlinker /implib:llvm-dump-test.lib -Xlinker /pdb:llvm-dump-test.pdb -Xlinker /version:0.0   -lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 -loldnames  &amp;&amp; cd ."
lld-link: error: undefined symbol: public: static void __cdecl llvm::ScaledNumberBase::dump(unsigned __int64, short, int)
&gt;&gt;&gt; referenced by S:\llvm\llvm-project\llvm\include\llvm\Support\ScaledNumber.h:614
&gt;&gt;&gt;               CMakeFiles/llvm-dump-test.dir/main.cxx.obj:(public: void __cdecl llvm::ScaledNumber&lt;unsigned __int64&gt;::dump(void) const)
clang++: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.

Verified the link issue is resolved after applying this change.

Also, manually included all header files modified in #139938 and ensured there were no other link errors.


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

2 Files Affected:

  • (modified) llvm/include/llvm/Support/ScaledNumber.h (+2-10)
  • (modified) llvm/lib/Support/ScaledNumber.cpp (+1-3)
diff --git a/llvm/include/llvm/Support/ScaledNumber.h b/llvm/include/llvm/Support/ScaledNumber.h
index 3d38677f0eb61..87a56809976a3 100644
--- a/llvm/include/llvm/Support/ScaledNumber.h
+++ b/llvm/include/llvm/Support/ScaledNumber.h
@@ -424,10 +424,7 @@ class ScaledNumberBase {
 public:
   static constexpr int DefaultPrecision = 10;
 
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
-  LLVM_DUMP_METHOD static void dump(uint64_t D, int16_t E, int Width);
-#endif
-
+  LLVM_ABI static void dump(uint64_t D, int16_t E, int Width);
   LLVM_ABI static raw_ostream &print(raw_ostream &OS, uint64_t D, int16_t E,
                                      int Width, unsigned Precision);
   LLVM_ABI static std::string toString(uint64_t D, int16_t E, int Width,
@@ -610,12 +607,7 @@ template <class DigitsT> class ScaledNumber : ScaledNumberBase {
                      unsigned Precision = DefaultPrecision) const {
     return ScaledNumberBase::print(OS, Digits, Scale, Width, Precision);
   }
-
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
-  LLVM_DUMP_METHOD void dump() const {
-    return ScaledNumberBase::dump(Digits, Scale, Width);
-  }
-#endif
+  void dump() const { return ScaledNumberBase::dump(Digits, Scale, Width); }
 
   ScaledNumber &operator+=(const ScaledNumber &X) {
     std::tie(Digits, Scale) =
diff --git a/llvm/lib/Support/ScaledNumber.cpp b/llvm/lib/Support/ScaledNumber.cpp
index 33e8cc3030873..85d7afbea5c69 100644
--- a/llvm/lib/Support/ScaledNumber.cpp
+++ b/llvm/lib/Support/ScaledNumber.cpp
@@ -317,9 +317,7 @@ raw_ostream &ScaledNumberBase::print(raw_ostream &OS, uint64_t D, int16_t E,
   return OS << toString(D, E, Width, Precision);
 }
 
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
-LLVM_DUMP_METHOD void ScaledNumberBase::dump(uint64_t D, int16_t E, int Width) {
+void ScaledNumberBase::dump(uint64_t D, int16_t E, int Width) {
   print(dbgs(), D, E, Width, 0) << "[" << Width << ":" << D << "*2^" << E
                                 << "]";
 }
-#endif

Copy link
Contributor

@hvdijk hvdijk left a comment

Choose a reason for hiding this comment

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

Thanks!

@compnerd compnerd merged commit dd0a1c5 into llvm:main May 19, 2025
13 checks passed
@andrurogerz andrurogerz deleted the dump-guards-fix branch July 23, 2025 19:41
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.

4 participants