https://github.com/snehasish updated https://github.com/llvm/llvm-project/pull/174895
>From 35297547967dcd51f88046cfba9093f7961a98fa Mon Sep 17 00:00:00 2001 From: Snehasish Kumar <[email protected]> Date: Wed, 7 Jan 2026 08:39:37 +0000 Subject: [PATCH 1/4] Add a flag to preserve the old macro behaviour. This allows us to more accurately quantify the impact of this change in isolation for sample based profiling which relies on debug information. --- clang/lib/CodeGen/CGDebugInfo.cpp | 33 ++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 9dcf8ccdec275..e0650a28d33bd 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -54,11 +54,17 @@ #include "llvm/Support/SHA1.h" #include "llvm/Support/SHA256.h" #include "llvm/Support/TimeProfiler.h" +#include "llvm/Support/CommandLine.h" #include <cstdint> #include <optional> using namespace clang; using namespace clang::CodeGen; +static llvm::cl::opt<bool> + UseOldDebugInfoLoc("use-old-debug-info-loc", + llvm::cl::desc("Use old expansion location for debug info"), + llvm::cl::init(false)); + static uint32_t getTypeAlignIfRequired(const Type *Ty, const ASTContext &Ctx) { auto TI = Ctx.getTypeInfo(Ty); if (TI.isAlignRequired()) @@ -345,7 +351,10 @@ void CGDebugInfo::setLocation(SourceLocation Loc) { if (Loc.isInvalid()) return; - CurLoc = CGM.getContext().getSourceManager().getFileLoc(Loc); + if (UseOldDebugInfoLoc) + CurLoc = CGM.getContext().getSourceManager().getExpansionLoc(Loc); + else + CurLoc = CGM.getContext().getSourceManager().getFileLoc(Loc); // If we've changed files in the middle of a lexical scope go ahead // and create a new lexical scope with file node if it's different @@ -572,7 +581,8 @@ llvm::DIFile *CGDebugInfo::getOrCreateFile(SourceLocation Loc) { FileName = TheCU->getFile()->getFilename(); CSInfo = TheCU->getFile()->getChecksum(); } else { - PresumedLoc PLoc = SM.getPresumedLoc(SM.getFileLoc(Loc)); + PresumedLoc PLoc = + SM.getPresumedLoc(UseOldDebugInfoLoc ? Loc : SM.getFileLoc(Loc)); FileName = PLoc.getFilename(); if (FileName.empty()) { @@ -599,8 +609,9 @@ llvm::DIFile *CGDebugInfo::getOrCreateFile(SourceLocation Loc) { if (CSKind) CSInfo.emplace(*CSKind, Checksum); } - return createFile(FileName, CSInfo, - getSource(SM, SM.getFileID(SM.getFileLoc(Loc)))); + return createFile( + FileName, CSInfo, + getSource(SM, SM.getFileID(UseOldDebugInfoLoc ? Loc : SM.getFileLoc(Loc)))); } llvm::DIFile *CGDebugInfo::createFile( @@ -655,7 +666,8 @@ unsigned CGDebugInfo::getLineNumber(SourceLocation Loc) { if (Loc.isInvalid()) return 0; SourceManager &SM = CGM.getContext().getSourceManager(); - return SM.getPresumedLoc(SM.getFileLoc(Loc)).getLine(); + return SM.getPresumedLoc(UseOldDebugInfoLoc ? Loc : SM.getFileLoc(Loc)) + .getLine(); } unsigned CGDebugInfo::getColumnNumber(SourceLocation Loc, bool Force) { @@ -667,8 +679,9 @@ unsigned CGDebugInfo::getColumnNumber(SourceLocation Loc, bool Force) { if (Loc.isInvalid() && CurLoc.isInvalid()) return 0; SourceManager &SM = CGM.getContext().getSourceManager(); - PresumedLoc PLoc = - SM.getPresumedLoc(Loc.isValid() ? SM.getFileLoc(Loc) : CurLoc); + PresumedLoc PLoc = SM.getPresumedLoc(Loc.isValid() + ? (UseOldDebugInfoLoc ? Loc : SM.getFileLoc(Loc)) + : CurLoc); return PLoc.isValid() ? PLoc.getColumn() : 0; } @@ -5014,7 +5027,8 @@ void CGDebugInfo::EmitLocation(CGBuilderTy &Builder, SourceLocation Loc) { // Update our current location setLocation(Loc); - if (CurLoc.isInvalid() || LexicalBlockStack.empty()) + if (CurLoc.isInvalid() || (UseOldDebugInfoLoc && CurLoc.isMacroID()) || + LexicalBlockStack.empty()) return; llvm::MDNode *Scope = LexicalBlockStack.back(); @@ -6282,7 +6296,8 @@ void CGDebugInfo::AddStringLiteralDebugInfo(llvm::GlobalVariable *GV, const StringLiteral *S) { SourceLocation Loc = S->getStrTokenLoc(0); SourceManager &SM = CGM.getContext().getSourceManager(); - PresumedLoc PLoc = SM.getPresumedLoc(SM.getFileLoc(Loc)); + PresumedLoc PLoc = + SM.getPresumedLoc(UseOldDebugInfoLoc ? Loc : SM.getFileLoc(Loc)); if (!PLoc.isValid()) return; >From 60f4958f0e1bdcd9a352bb6a82934922d4697ae6 Mon Sep 17 00:00:00 2001 From: Snehasish Kumar <[email protected]> Date: Wed, 7 Jan 2026 21:34:36 +0000 Subject: [PATCH 2/4] [Clang][DebugInfo] Add a flag to use expansion loc for macro params. This patch adds a flag to allow users to preserve the old behaviour - use the macro expansion location for parameters. This is useful for wider testing of sample profile driven PGO which relies on debug information based mapping. --- clang/lib/CodeGen/CGDebugInfo.cpp | 35 ++++++++++++----------- clang/test/DebugInfo/Generic/macro-info.c | 18 ++++++++---- 2 files changed, 31 insertions(+), 22 deletions(-) diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index e0650a28d33bd..e9ab08d11e299 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -60,10 +60,10 @@ using namespace clang; using namespace clang::CodeGen; -static llvm::cl::opt<bool> - UseOldDebugInfoLoc("use-old-debug-info-loc", - llvm::cl::desc("Use old expansion location for debug info"), - llvm::cl::init(false)); +static llvm::cl::opt<bool> DebugInfoMacroExpansionLoc( + "debug-info-macro-expansion-loc", + llvm::cl::desc("Use expansion location for debug info on macro params"), + llvm::cl::init(false)); static uint32_t getTypeAlignIfRequired(const Type *Ty, const ASTContext &Ctx) { auto TI = Ctx.getTypeInfo(Ty); @@ -351,7 +351,7 @@ void CGDebugInfo::setLocation(SourceLocation Loc) { if (Loc.isInvalid()) return; - if (UseOldDebugInfoLoc) + if (DebugInfoMacroExpansionLoc) CurLoc = CGM.getContext().getSourceManager().getExpansionLoc(Loc); else CurLoc = CGM.getContext().getSourceManager().getFileLoc(Loc); @@ -581,8 +581,8 @@ llvm::DIFile *CGDebugInfo::getOrCreateFile(SourceLocation Loc) { FileName = TheCU->getFile()->getFilename(); CSInfo = TheCU->getFile()->getChecksum(); } else { - PresumedLoc PLoc = - SM.getPresumedLoc(UseOldDebugInfoLoc ? Loc : SM.getFileLoc(Loc)); + PresumedLoc PLoc = SM.getPresumedLoc( + DebugInfoMacroExpansionLoc ? Loc : SM.getFileLoc(Loc)); FileName = PLoc.getFilename(); if (FileName.empty()) { @@ -609,9 +609,10 @@ llvm::DIFile *CGDebugInfo::getOrCreateFile(SourceLocation Loc) { if (CSKind) CSInfo.emplace(*CSKind, Checksum); } - return createFile( - FileName, CSInfo, - getSource(SM, SM.getFileID(UseOldDebugInfoLoc ? Loc : SM.getFileLoc(Loc)))); + return createFile(FileName, CSInfo, + getSource(SM, SM.getFileID(DebugInfoMacroExpansionLoc + ? Loc + : SM.getFileLoc(Loc)))); } llvm::DIFile *CGDebugInfo::createFile( @@ -666,7 +667,8 @@ unsigned CGDebugInfo::getLineNumber(SourceLocation Loc) { if (Loc.isInvalid()) return 0; SourceManager &SM = CGM.getContext().getSourceManager(); - return SM.getPresumedLoc(UseOldDebugInfoLoc ? Loc : SM.getFileLoc(Loc)) + return SM + .getPresumedLoc(DebugInfoMacroExpansionLoc ? Loc : SM.getFileLoc(Loc)) .getLine(); } @@ -679,9 +681,9 @@ unsigned CGDebugInfo::getColumnNumber(SourceLocation Loc, bool Force) { if (Loc.isInvalid() && CurLoc.isInvalid()) return 0; SourceManager &SM = CGM.getContext().getSourceManager(); - PresumedLoc PLoc = SM.getPresumedLoc(Loc.isValid() - ? (UseOldDebugInfoLoc ? Loc : SM.getFileLoc(Loc)) - : CurLoc); + PresumedLoc PLoc = SM.getPresumedLoc( + Loc.isValid() ? (DebugInfoMacroExpansionLoc ? Loc : SM.getFileLoc(Loc)) + : CurLoc); return PLoc.isValid() ? PLoc.getColumn() : 0; } @@ -5027,7 +5029,8 @@ void CGDebugInfo::EmitLocation(CGBuilderTy &Builder, SourceLocation Loc) { // Update our current location setLocation(Loc); - if (CurLoc.isInvalid() || (UseOldDebugInfoLoc && CurLoc.isMacroID()) || + if (CurLoc.isInvalid() || + (DebugInfoMacroExpansionLoc && CurLoc.isMacroID()) || LexicalBlockStack.empty()) return; @@ -6297,7 +6300,7 @@ void CGDebugInfo::AddStringLiteralDebugInfo(llvm::GlobalVariable *GV, SourceLocation Loc = S->getStrTokenLoc(0); SourceManager &SM = CGM.getContext().getSourceManager(); PresumedLoc PLoc = - SM.getPresumedLoc(UseOldDebugInfoLoc ? Loc : SM.getFileLoc(Loc)); + SM.getPresumedLoc(DebugInfoMacroExpansionLoc ? Loc : SM.getFileLoc(Loc)); if (!PLoc.isValid()) return; diff --git a/clang/test/DebugInfo/Generic/macro-info.c b/clang/test/DebugInfo/Generic/macro-info.c index ec49eb5d65f9c..3f47050e60317 100644 --- a/clang/test/DebugInfo/Generic/macro-info.c +++ b/clang/test/DebugInfo/Generic/macro-info.c @@ -1,4 +1,5 @@ -// RUN: %clang_cc1 %s -debug-info-kind=standalone -emit-llvm -o - | FileCheck %s +// RUN: %clang_cc1 %s -debug-info-kind=standalone -emit-llvm -o - | FileCheck %s --check-prefix=NEW +// RUN: %clang_cc1 %s -debug-info-kind=standalone -emit-llvm -mllvm -debug-info-macro-expansion-loc -o - | FileCheck %s --check-prefix=OLD #define GLOBAL(num) global## num #define DECL_GLOBAL(x) int x @@ -9,11 +10,13 @@ SAME_ORDER( int -// CHECK: DIGlobalVariable(name: "global",{{.*}} line: [[@LINE+1]] +// NEW: DIGlobalVariable(name: "global",{{.*}} line: [[@LINE+2]] +// OLD: DIGlobalVariable(name: "global",{{.*}} line: [[@LINE-3]] GLOBAL // <- global () = 42, const char* s() { -// CHECK: DIGlobalVariable({{.*}}line: [[@LINE+1]],{{.*}} type: [[TYPEID:![0-9]+]] +// NEW: DIGlobalVariable({{.*}}line: [[@LINE+2]],{{.*}} type: [[TYPEID:![0-9]+]] +// OLD: DIGlobalVariable({{.*}}line: [[@LINE-8]],{{.*}} type: [[TYPEID:![0-9]+]] return "1234567890"; } ) @@ -21,8 +24,10 @@ SAME_ORDER( SWAP_ORDER( int GLOBAL( // <- global2 2) = 43, -// CHECK: DIGlobalVariable(name: "global3",{{.*}} line: [[@LINE+3]] -// CHECK: DIGlobalVariable(name: "global2",{{.*}} line: [[@LINE-3]] +// NEW: DIGlobalVariable(name: "global3",{{.*}} line: [[@LINE+5]] +// NEW: DIGlobalVariable(name: "global2",{{.*}} line: [[@LINE-3]] +// OLD: DIGlobalVariable(name: "global3",{{.*}} line: [[@LINE-5]] +// OLD: DIGlobalVariable(name: "global2",{{.*}} line: [[@LINE-6]] DECL_GLOBAL( GLOBAL( // <- global3 3)) = 44 @@ -30,6 +35,7 @@ SWAP_ORDER( DECL_GLOBAL( -// CHECK: DIGlobalVariable(name: "global4",{{.*}} line: [[@LINE+1]] +// NEW: DIGlobalVariable(name: "global4",{{.*}} line: [[@LINE+2]] +// OLD: DIGlobalVariable(name: "global4",{{.*}} line: [[@LINE-2]] GLOBAL( // <- global4 4)); >From be6c875eac3f9969fe662b4cf93dc3394872e3b2 Mon Sep 17 00:00:00 2001 From: Snehasish Kumar <[email protected]> Date: Thu, 8 Jan 2026 23:02:14 +0000 Subject: [PATCH 3/4] Address comments --- clang/include/clang/Basic/DebugOptions.def | 3 ++ clang/include/clang/Options/Options.td | 4 +++ clang/lib/CodeGen/CGDebugInfo.cpp | 36 +++++++++------------- clang/test/DebugInfo/Generic/macro-info.c | 2 +- 4 files changed, 22 insertions(+), 23 deletions(-) diff --git a/clang/include/clang/Basic/DebugOptions.def b/clang/include/clang/Basic/DebugOptions.def index 34f5a313947a4..c41f2d93c49f9 100644 --- a/clang/include/clang/Basic/DebugOptions.def +++ b/clang/include/clang/Basic/DebugOptions.def @@ -100,6 +100,9 @@ ENUM_DEBUGOPT(DebugInfo, DebugInfoKind, 4, /// Whether to generate macro debug info. DEBUGOPT(MacroDebugInfo, 1, 0, Compatible) +/// Whether to use expansion location for debug info. +DEBUGOPT(DebugInfoMacroExpansionLoc, 1, 0, Compatible) + /// Tune the debug info for this debugger. ENUM_DEBUGOPT(DebuggerTuning, DebuggerKind, 3, DebuggerKind::Default, Compatible) diff --git a/clang/include/clang/Options/Options.td b/clang/include/clang/Options/Options.td index 006a300d4cf6b..6a72931727a7c 100644 --- a/clang/include/clang/Options/Options.td +++ b/clang/include/clang/Options/Options.td @@ -7812,6 +7812,10 @@ let Visibility = [CC1Option, CC1AsOption] in { def debug_info_macro : Flag<["-"], "debug-info-macro">, HelpText<"Emit macro debug information">, MarshallingInfoFlag<CodeGenOpts<"MacroDebugInfo">>; +def debug_info_macro_expansion_loc + : Flag<["-"], "debug-info-macro-expansion-loc">, + HelpText<"Use expansion location for debug info for multi line macros">, + MarshallingInfoFlag<CodeGenOpts<"DebugInfoMacroExpansionLoc">>; def default_function_attr : Separate<["-"], "default-function-attr">, HelpText<"Apply given attribute to all functions">, MarshallingInfoStringVector<CodeGenOpts<"DefaultFunctionAttrs">>; diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index e9ab08d11e299..0576944cd6877 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -54,16 +54,17 @@ #include "llvm/Support/SHA1.h" #include "llvm/Support/SHA256.h" #include "llvm/Support/TimeProfiler.h" -#include "llvm/Support/CommandLine.h" #include <cstdint> #include <optional> using namespace clang; using namespace clang::CodeGen; -static llvm::cl::opt<bool> DebugInfoMacroExpansionLoc( - "debug-info-macro-expansion-loc", - llvm::cl::desc("Use expansion location for debug info on macro params"), - llvm::cl::init(false)); +static SourceLocation getMacroDebugLoc(const CodeGenModule &CGM, + SourceLocation Loc) { + if (CGM.getCodeGenOpts().DebugInfoMacroExpansionLoc) + return Loc; + return CGM.getContext().getSourceManager().getFileLoc(Loc); +} static uint32_t getTypeAlignIfRequired(const Type *Ty, const ASTContext &Ctx) { auto TI = Ctx.getTypeInfo(Ty); @@ -351,10 +352,8 @@ void CGDebugInfo::setLocation(SourceLocation Loc) { if (Loc.isInvalid()) return; - if (DebugInfoMacroExpansionLoc) - CurLoc = CGM.getContext().getSourceManager().getExpansionLoc(Loc); - else - CurLoc = CGM.getContext().getSourceManager().getFileLoc(Loc); + CurLoc = CGM.getContext().getSourceManager().getExpansionLoc( + getMacroDebugLoc(CGM, Loc)); // If we've changed files in the middle of a lexical scope go ahead // and create a new lexical scope with file node if it's different @@ -581,8 +580,7 @@ llvm::DIFile *CGDebugInfo::getOrCreateFile(SourceLocation Loc) { FileName = TheCU->getFile()->getFilename(); CSInfo = TheCU->getFile()->getChecksum(); } else { - PresumedLoc PLoc = SM.getPresumedLoc( - DebugInfoMacroExpansionLoc ? Loc : SM.getFileLoc(Loc)); + PresumedLoc PLoc = SM.getPresumedLoc(getMacroDebugLoc(CGM, Loc)); FileName = PLoc.getFilename(); if (FileName.empty()) { @@ -610,9 +608,7 @@ llvm::DIFile *CGDebugInfo::getOrCreateFile(SourceLocation Loc) { CSInfo.emplace(*CSKind, Checksum); } return createFile(FileName, CSInfo, - getSource(SM, SM.getFileID(DebugInfoMacroExpansionLoc - ? Loc - : SM.getFileLoc(Loc)))); + getSource(SM, SM.getFileID(getMacroDebugLoc(CGM, Loc)))); } llvm::DIFile *CGDebugInfo::createFile( @@ -667,9 +663,7 @@ unsigned CGDebugInfo::getLineNumber(SourceLocation Loc) { if (Loc.isInvalid()) return 0; SourceManager &SM = CGM.getContext().getSourceManager(); - return SM - .getPresumedLoc(DebugInfoMacroExpansionLoc ? Loc : SM.getFileLoc(Loc)) - .getLine(); + return SM.getPresumedLoc(getMacroDebugLoc(CGM, Loc)).getLine(); } unsigned CGDebugInfo::getColumnNumber(SourceLocation Loc, bool Force) { @@ -682,8 +676,7 @@ unsigned CGDebugInfo::getColumnNumber(SourceLocation Loc, bool Force) { return 0; SourceManager &SM = CGM.getContext().getSourceManager(); PresumedLoc PLoc = SM.getPresumedLoc( - Loc.isValid() ? (DebugInfoMacroExpansionLoc ? Loc : SM.getFileLoc(Loc)) - : CurLoc); + Loc.isValid() ? getMacroDebugLoc(CGM, Loc) : CurLoc); return PLoc.isValid() ? PLoc.getColumn() : 0; } @@ -5030,7 +5023,7 @@ void CGDebugInfo::EmitLocation(CGBuilderTy &Builder, SourceLocation Loc) { setLocation(Loc); if (CurLoc.isInvalid() || - (DebugInfoMacroExpansionLoc && CurLoc.isMacroID()) || + (CGM.getCodeGenOpts().DebugInfoMacroExpansionLoc && CurLoc.isMacroID()) || LexicalBlockStack.empty()) return; @@ -6299,8 +6292,7 @@ void CGDebugInfo::AddStringLiteralDebugInfo(llvm::GlobalVariable *GV, const StringLiteral *S) { SourceLocation Loc = S->getStrTokenLoc(0); SourceManager &SM = CGM.getContext().getSourceManager(); - PresumedLoc PLoc = - SM.getPresumedLoc(DebugInfoMacroExpansionLoc ? Loc : SM.getFileLoc(Loc)); + PresumedLoc PLoc = SM.getPresumedLoc(getMacroDebugLoc(CGM, Loc)); if (!PLoc.isValid()) return; diff --git a/clang/test/DebugInfo/Generic/macro-info.c b/clang/test/DebugInfo/Generic/macro-info.c index 3f47050e60317..8d9d2502be03f 100644 --- a/clang/test/DebugInfo/Generic/macro-info.c +++ b/clang/test/DebugInfo/Generic/macro-info.c @@ -1,5 +1,5 @@ // RUN: %clang_cc1 %s -debug-info-kind=standalone -emit-llvm -o - | FileCheck %s --check-prefix=NEW -// RUN: %clang_cc1 %s -debug-info-kind=standalone -emit-llvm -mllvm -debug-info-macro-expansion-loc -o - | FileCheck %s --check-prefix=OLD +// RUN: %clang_cc1 %s -debug-info-kind=standalone -emit-llvm -debug-info-macro-expansion-loc -o - | FileCheck %s --check-prefix=OLD #define GLOBAL(num) global## num #define DECL_GLOBAL(x) int x >From aaab3dff264c15945fb2214238175c03f5653710 Mon Sep 17 00:00:00 2001 From: Snehasish Kumar <[email protected]> Date: Fri, 9 Jan 2026 00:01:15 +0000 Subject: [PATCH 4/4] clang format --- clang/lib/CodeGen/CGDebugInfo.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 0576944cd6877..d63d4c59382b6 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -675,8 +675,8 @@ unsigned CGDebugInfo::getColumnNumber(SourceLocation Loc, bool Force) { if (Loc.isInvalid() && CurLoc.isInvalid()) return 0; SourceManager &SM = CGM.getContext().getSourceManager(); - PresumedLoc PLoc = SM.getPresumedLoc( - Loc.isValid() ? getMacroDebugLoc(CGM, Loc) : CurLoc); + PresumedLoc PLoc = + SM.getPresumedLoc(Loc.isValid() ? getMacroDebugLoc(CGM, Loc) : CurLoc); return PLoc.isValid() ? PLoc.getColumn() : 0; } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
