Strip the full path from __FILE__ in the LDBG macro and keep only the filename #150677
+29
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-llvm-support Author: Mehdi Amini (joker-eph) ChangesFull diff: https://github.com/llvm/llvm-project/pull/150677.diff 1 Files Affected:
diff --git a/llvm/include/llvm/Support/DebugLog.h b/llvm/include/llvm/Support/DebugLog.h
index 9556bf2d6242d..a93aa54a54bde 100644
--- a/llvm/include/llvm/Support/DebugLog.h
+++ b/llvm/include/llvm/Support/DebugLog.h
@@ -29,7 +29,15 @@ namespace llvm {
#define DEBUGLOG_WITH_STREAM_AND_TYPE(STREAM, TYPE) \
for (bool _c = (::llvm::DebugFlag && ::llvm::isCurrentDebugType(TYPE)); _c; \
_c = false) \
- ::llvm::impl::LogWithNewline(TYPE, __FILE__, __LINE__, (STREAM))
+ ::llvm::impl::LogWithNewline( \
+ TYPE, \
+ [] { \
+ /* Force constexpr eval */ \
+ constexpr const char *filename = \
+ ::llvm::impl::LogWithNewline::getFileName(__FILE__); \
+ return filename; \
+ }(), \
+ __LINE__, (STREAM))
namespace impl {
class LogWithNewline {
@@ -51,6 +59,16 @@ class LogWithNewline {
LogWithNewline(const LogWithNewline &) = delete;
LogWithNewline &operator=(const LogWithNewline &) = delete;
LogWithNewline &operator=(LogWithNewline &&) = delete;
+ static constexpr const char *getFileName(const char *path) {
+ // Remove the path prefix from the file name.
+ const char *filename = path;
+ for (const char *p = path; *p != '\0'; ++p) {
+ if (*p == '/' || *p == '\\') {
+ filename = p + 1;
+ }
+ }
+ return filename;
+ }
private:
raw_ostream &os;
|
jpienaar
approved these changes
Jul 25, 2025
llvm/include/llvm/Support/DebugLog.h
Outdated
| ::llvm::impl::LogWithNewline(TYPE, __FILE__, __LINE__, (STREAM)) | ||
| ::llvm::impl::LogWithNewline( \ | ||
| TYPE, \ | ||
| [] { \ |
There was a problem hiding this comment.
Could we do this instead in the constructor for LogWithNewline ?
There was a problem hiding this comment.
I don't think it can be constexpr evaluated in this case?
(this is why I put it in the macro in the first place)
include/llvm/Support/DebugLog.h:48:27: error: constexpr variable 'filename' must be initialized by a constant expression
48 | constexpr const char *filename =
| ^
49 | ::llvm::impl::LogWithNewline::getFileName(file);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/llvm/Support/DebugLog.h:49:49: note: function parameter 'file' with unknown value cannot be used in a constant expression
49 | ::llvm::impl::LogWithNewline::getFileName(file);
| ^
include/llvm/Support/DebugLog.h:45:64: note: declared here
45 | constexpr LogWithNewline(const char *debug_type, const char *file, int line,
| ^
1 error generated.
I think this may matter in debug builds.
joker-eph
force-pushed
the
ldb_strip_path
branch
from
90360ed to
e81bdd6
Compare
joker-eph
force-pushed
the
ldb_strip_path
branch
from
e81bdd6 to
2695936
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/11/builds/20430 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/13673 Here is the relevant piece of the build log for the reference |
mahesh-attarde
pushed a commit
to mahesh-attarde/llvm-project
that referenced
this pull request
Jul 28, 2025
|
@joker-eph After this cmake change msvc bullds have crawled to a halt - typically building on only a couple of cores at a time and with no build distribution at all :( |
|
@RKSimon ouch! I don't have a windows machine, I assume this is need to debug this? I looked at some windows bots but they don't seem to be affected (e.g. My change on this bot: https://lab.llvm.org/buildbot/#/builders/63/builds/8184 ), but it's always to tell with incremental builds. This change has two components:
|
|
Its msbuild builds that seem to be affected, not ninja+msvc which is what I think all the buildbots use. I've deleted and rebuilt the build directories and they are still affected (and I ended up bisecting to find this so was regenerating a lot in the process), seen on both Debug and RelWithDebInfo builds so far. |
|
As a temp hack can we just disable the |
Oh see my ignorance, I don't even know what is "msbuild" 🫣
Sure, can you send a PR that works for you? |
RKSimon
added a commit
to RKSimon/llvm-project
that referenced
this pull request
Jul 29, 2025
As reported on llvm#150677 - this prevents build parallelisation as cmake is repeatedly updating the cache
|
For what it's worth, I bisected my At the parent change, there is no issue. $ ninja -C build clang
ninja: Entering directory `build'
[2916/2916] Creating executable symlink bin/clang
$ build/bin/clang --version | head -1
ClangBuiltLinux clang version 22.0.0git (https://github.com/llvm/llvm-project.git 5ec6ac882c9fffaae6cf194f738e86f796394cd3) |
|
Thanks for the report! It should be fixed with: #151251 |
RKSimon
added a commit
to RKSimon/llvm-project
that referenced
this pull request
Jul 30, 2025
As reported on llvm#150677 - this prevents build parallelisation as cmake is repeatedly updating the cache
|
@joker-eph I think this completely breaks clang module build since a different macro for each source file means the modules are never shared anymore. This causes https://green.lab.llvm.org/job/clang-stage2-Rthinlto/ running out of space (and really really slow). Also, why we don't use |
jroelofs
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Aug 6, 2025
…only the filename (llvm#150677)" This reverts commit 5d26e3c. It breaks the modules build of clang, since every source file has a different version of this macro, which causes https://green.lab.llvm.org/job/clang-stage2-Rthinlto/ to run out of space. We should probably be using __FILE_NAME__ instead when the host compiler supports it, but until then let's revert-to-green, and un-block some of the bots. see: llvm#150677 see: llvm#150677 (comment)
jroelofs
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Aug 6, 2025
…only the filename (llvm#150677)" This reverts commit 5d26e3c. It breaks the modules build of clang, since every source file has a different version of this macro, which causes https://green.lab.llvm.org/job/clang-stage2-Rthinlto/ to run out of space. We should probably be using __FILE_NAME__ instead when the host compiler supports it, but until then let's revert-to-green, and un-block some of the bots. see: llvm#150677 see: llvm#150677 (comment) rdar://157465825
jroelofs
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Aug 6, 2025
…only the filename (llvm#150677)" This reverts commit 5d26e3c. It breaks the modules build of clang, since every source file has a different version of this macro, which causes https://green.lab.llvm.org/job/clang-stage2-Rthinlto/ to run out of space. We should probably be using __FILE_NAME__ instead when the host compiler supports it, but until then let's revert-to-green, and un-block some of the bots. see: llvm#150677 see: llvm#150677 (comment) rdar://157465825
jroelofs
added a commit
that referenced
this pull request
Aug 6, 2025
…only the filename (#150677)" This reverts commit 5d26e3c. It breaks the modules build of clang, since every source file has a different version of this macro, which causes https://green.lab.llvm.org/job/clang-stage2-Rthinlto/ to run out of space. We should probably be using __FILE_NAME__ instead when the host compiler supports it, but until then let's revert-to-green, and un-block the bots. see: #150677 see: #150677 (comment) rdar://157465825
|
I reverted this in a15b629 to get https://green.lab.llvm.org/job/clang-stage2-Rthinlto/ back to green (disabled currently, as it was trashing disk space on all the nodes in the label). |
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Aug 6, 2025
…o and keep only the filename (#150677)" This reverts commit 5d26e3c. It breaks the modules build of clang, since every source file has a different version of this macro, which causes https://green.lab.llvm.org/job/clang-stage2-Rthinlto/ to run out of space. We should probably be using __FILE_NAME__ instead when the host compiler supports it, but until then let's revert-to-green, and un-block the bots. see: llvm/llvm-project#150677 see: llvm/llvm-project#150677 (comment) rdar://157465825
I'm not sure what does that mean, can you explain a bit more?
I could work something out that would only work with gcc and clang, and let MSVC take the slow path. Thanks @jroelofs for reverting in the meantime, and sorry for the breakage! It does not seem like an easy issue to root cause. |
I thought we were going to have to take some more drastic measures to bisect this, and wrote up a whole proposal for how to re-architect GreenDragon jobs for automatic bisection: llvm/llvm-zorg#533. That said, we ended up finding it without that after seeing it hit multiple buildbots and intersecting their commit lists. This commit stuck out to me, but I didn't realize why until it clicked for @cachemeifyoucan. Anyway, long-winded way of saying: if you have some time to review my bisection PR, that would help for next time ;) p.s: I had to revert the revert in f9386d3. Did it too hastily, and broke more things.
@cachemeifyoucan @joker-eph how about: |
joker-eph
added a commit
to joker-eph/llvm-project
that referenced
this pull request
Aug 6, 2025
This per-file macro definition on the command line breaks caching of modules. See discussion in llvm#150677 Instead we use a constexpr variable to force the constexpr evaluation by the frontend, and also the __FILE_NAME__ macro when available (clang/gcc) to spare compile-time in the frontend.
joker-eph
added a commit
to joker-eph/llvm-project
that referenced
this pull request
Aug 6, 2025
This per-file macro definition on the command line breaks caching of modules. See discussion in llvm#150677 Instead we use a constexpr variable to force the constexpr evaluation by the frontend, and also the __FILE_NAME__ macro when available (clang/gcc) to spare compile-time in the frontend.
|
@RKSimon do you know if your MSBuild config was using modules? (checking if this is related somehow) |
|
No, it was a very basic cmake build, llvm+clang projects with assertions and EXPENSIVE_CHECKS. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.