[clang] [lldb] [llvm] Remove some `try_compile` CMake checks for compiler flags (PR #92953)
felipepiovezan wrote: These types of changes touching a lot of projects at once can benefit from multiple PRs, one per project, as it makes partial reverts a lot easier and doesn't cause as much churn downstream (plus we can get more targeted comments from individual project owners) https://github.com/llvm/llvm-project/pull/92953 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [compiler-rt] [flang] [libc] [libclc] [libcxx] [libcxxabi] [libunwind] [lld] [lldb] [llvm] [mlir] [openmp] [polly] [pstl] Update IDE Folders (PR #89153)
@@ -406,5 +426,13 @@ function(llvm_ExternalProject_Add name source_dir) WORKING_DIRECTORY ${BINARY_DIR} VERBATIM USES_TERMINAL) +if (ARG_FOLDER) + set_target_properties(${target} PROPERTIES FOLDER "${ARG_FOLDER}") +endif () endforeach() + + #set_target_folder( + # TARGETS ${name} install-${name} ${name}-clobber ${name}-clear ${name}-clean ${ARG_EXTRA_TARGETS} + # FOLDER "${ARG_FOLDE}" + #) felipepiovezan wrote: commented code? https://github.com/llvm/llvm-project/pull/89153 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [compiler-rt] [flang] [libclc] [libcxx] [lld] [lldb] [llvm] [NFC] Remove trailing whitespace across all non-test related files (PR #82838)
felipepiovezan wrote: > > This seems akin to running clang-format on the entire project, which last > > time we talked about still faced opposition > > My impression (I admit I haven't reviewed the whole thread lately) is that > the opposition has mostly to do with how clang-format mangles some > constructs, not to the idea in general. > > I'm juggling 4 projects at the moment, I have to finish some internal > release-related work before I can get back to clang-format-all-the-things. Sorry, you might be right; my intent with that comment was not to a make broad summary of the thread, but rather to point out the thread's existence and that this PR should probably be folded into that discussion. https://github.com/llvm/llvm-project/pull/82838 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [compiler-rt] [flang] [libclc] [libcxx] [lld] [lldb] [llvm] [NFC] Remove trailing whitespace across all non-test related files (PR #82838)
felipepiovezan wrote: Juuust to make sure, was this discussed before? This seems akin to running clang-format on the entire project, which last time we talked about still faced opposition: https://discourse.llvm.org/t/rfc-clang-format-all-the-things/76614/11 https://github.com/llvm/llvm-project/pull/82838 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Add support for builtin_verbose_trap (PR #79230)
https://github.com/felipepiovezan edited https://github.com/llvm/llvm-project/pull/79230 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Add support for builtin_verbose_trap (PR #79230)
@@ -3416,6 +3437,27 @@ llvm::DIMacroFile *CGDebugInfo::CreateTempMacroFile(llvm::DIMacroFile *Parent, return DBuilder.createTempMacroFile(Parent, Line, FName); } +llvm::DILocation *CGDebugInfo::CreateTrapFailureMessageFor( +llvm::DebugLoc TrapLocation, StringRef Prefix, StringRef FailureMsg) { + // Create debug info that describes a fake function whose name is the failure + // message. + std::string FuncName(Prefix); + if (!FailureMsg.empty()) { +// A space in the function name identifies this as not being a real function +// because it's not a valid symbol name. +FuncName += ": "; +FuncName += FailureMsg; + } + + assert(FuncName.size() > 0); + assert(FuncName.find(' ') != std::string::npos && felipepiovezan wrote: We can better express this intent by moving the assert _before_ the `if` and by rewriting it as the faster `assert(!FailureMessage.empty() || FuncName.find(' ') != npos)` https://github.com/llvm/llvm-project/pull/79230 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Add support for builtin_verbose_trap (PR #79230)
https://github.com/felipepiovezan edited https://github.com/llvm/llvm-project/pull/79230 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Add support for builtin_verbose_trap (PR #79230)
@@ -3416,6 +3437,27 @@ llvm::DIMacroFile *CGDebugInfo::CreateTempMacroFile(llvm::DIMacroFile *Parent, return DBuilder.createTempMacroFile(Parent, Line, FName); } +llvm::DILocation *CGDebugInfo::CreateTrapFailureMessageFor( +llvm::DebugLoc TrapLocation, StringRef Prefix, StringRef FailureMsg) { + // Create debug info that describes a fake function whose name is the failure + // message. + std::string FuncName(Prefix); + if (!FailureMsg.empty()) { +// A space in the function name identifies this as not being a real function +// because it's not a valid symbol name. +FuncName += ": "; +FuncName += FailureMsg; + } + + assert(FuncName.size() > 0); + assert(FuncName.find(' ') != std::string::npos && felipepiovezan wrote: the pre condition (as stated in the function documentation) is that `Prefix` should have a space. But we add a space whenever `FailureMsg` is not empty, so this assert is not catching a pre condition violation in all cases https://github.com/llvm/llvm-project/pull/79230 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[lldb] [clang] fixing issue #64441 (PR #74814)
https://github.com/felipepiovezan requested changes to this pull request. Hi @jeevanghimire, thank you for the PR! I believe the original issue was not about changing LLVM test inputs, but rather about changing clang-tidy to warn about this kind of patterns. Please see my rationale in a previous comment for why we should not be changing tests. For future contributions, I'd also recommend using a more descriptive commit title and message. If you inspect the git log of the project, we generally follow the pattern: [some topic] For example, this commit message could have been: [clang-tidy] Implement "using namespace" check https://github.com/llvm/llvm-project/pull/74814 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lldb] fixing issue #64441 (PR #74814)
@@ -34,7 +34,7 @@ namespace A { int myfunc (int a); int myfunc2(int a) { - return a + 2; +return a + 2; //just changing tab not much felipepiovezan wrote: In general, we don't add diffs that are unrelated to the problem being solved by the PR. On a similar note, this comment seems to be trying to communicate with reviewers, not future readers of this test file. As such, this is the kind of comment that should be part of the commit message / PR description, and not part of the test itself https://github.com/llvm/llvm-project/pull/74814 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lldb] fixing issue #64441 (PR #74814)
@@ -56,10 +56,10 @@ namespace Foo = A::B; // namespace alias using Foo::myfunc; // using declaration -using namespace Foo;// using directive +//removing namespace foo; for quality naming felipepiovezan wrote: Hi @jeevanghimire, please note that this is a test, so we must ensure that LLDB does the right thing regardless of the input it receives, even if said input is not considered best practices. The test was likely created to either ensure LLDB works in the case, or because it didn't use to work and someone created a test at the same time the fix was done. As such, I don't think we should change this. https://github.com/llvm/llvm-project/pull/74814 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Avoid memcopy for small structure with padding under -ftrivial-auto-var-init (PR #71677)
felipepiovezan wrote: If this gets reverted for some reason, please note that I had to update a cross-project-tests test: https://github.com/llvm/llvm-project/pull/73566 https://github.com/llvm/llvm-project/pull/71677 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][DebugInfo] Fix iterator invalidation during EmitGlobalVariable (PR #72415)
https://github.com/felipepiovezan edited https://github.com/llvm/llvm-project/pull/72415 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][DebugInfo] Fix iterator invalidation during EmitGlobalVariable (PR #72415)
https://github.com/felipepiovezan approved this pull request. LGTM. If there are others waiting on the fix, and since this is a fairly trivial change, you may want to push it soonish https://github.com/llvm/llvm-project/pull/72415 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Add RunTimeLang to Class and Enumeration DIBuilder functions (PR #72011)
@@ -424,6 +424,7 @@ namespace llvm { /// \param OffsetInBits Member offset. /// \param FlagsFlags to encode member attribute, e.g. private /// \param Elements class members. +/// \param RunTimeLang Optional parameter, Objective-C runtime version. felipepiovezan wrote: Ah, that's fair then https://github.com/llvm/llvm-project/pull/72011 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Add RunTimeLang to Class and Enumeration DIBuilder functions (PR #72011)
felipepiovezan wrote: > @felipepiovezan I don't this patch as it is has any changes that are > testable. I haven't added any code that will emit a RuntimeLang, only added > the option to do that so I can add those callers downstream. In retrospect I > should've marked it as NFC. I see. I think this is yet another argument for flipping the argument order: if the new thing were the last argument, it would have been even more evident this is NFC :) https://github.com/llvm/llvm-project/pull/72011 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] Add RunTimeLang to Class and Enumeration DIBuilder functions (PR #72011)
https://github.com/felipepiovezan edited https://github.com/llvm/llvm-project/pull/72011 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Add RunTimeLang to Class and Enumeration DIBuilder functions (PR #72011)
https://github.com/felipepiovezan edited https://github.com/llvm/llvm-project/pull/72011 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Add RunTimeLang to Class and Enumeration DIBuilder functions (PR #72011)
@@ -424,6 +424,7 @@ namespace llvm { /// \param OffsetInBits Member offset. /// \param FlagsFlags to encode member attribute, e.g. private /// \param Elements class members. +/// \param RunTimeLang Optional parameter, Objective-C runtime version. felipepiovezan wrote: let's use an `optional` type to denote an optional value https://github.com/llvm/llvm-project/pull/72011 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Add RunTimeLang to Class and Enumeration DIBuilder functions (PR #72011)
@@ -424,6 +424,7 @@ namespace llvm { /// \param OffsetInBits Member offset. /// \param FlagsFlags to encode member attribute, e.g. private /// \param Elements class members. +/// \param RunTimeLang Optional parameter, Objective-C runtime version. felipepiovezan wrote: I think your entire patch would be a lot simpler if this were the last parameter of the function, as you wouldn't need to change as many callsites and have inline comments explaining the parameter. Did you have a motivation in mind for placing the argument here? https://github.com/llvm/llvm-project/pull/72011 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] Add RunTimeLang to Class and Enumeration DIBuilder functions (PR #72011)
https://github.com/felipepiovezan edited https://github.com/llvm/llvm-project/pull/72011 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] Add RunTimeLang to Class and Enumeration DIBuilder functions (PR #72011)
https://github.com/felipepiovezan commented: Let's add the `[DebugInfo][CGDebugInfo]` tags the commit message so that the git log is more searchable. I think we should have at least one test in this patch as well https://github.com/llvm/llvm-project/pull/72011 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][DebugInfo][NFC] Add createConstantValueExpression helper (PR #70674)
@@ -5935,3 +5918,28 @@ llvm::DINode::DIFlags CGDebugInfo::getCallSiteRelatedAttrs() const { return llvm::DINode::FlagAllCallsDescribed; } + +llvm::DIExpression * +CGDebugInfo::createConstantValueExpression(clang::ValueDecl const *VD, + const APValue ) { felipepiovezan wrote: `const` is a inconsistent here, right? We have both east and west const https://github.com/llvm/llvm-project/pull/70674 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][DebugInfo][NFC] Add createConstantValueExpression helper (PR #70674)
@@ -5935,3 +5918,28 @@ llvm::DINode::DIFlags CGDebugInfo::getCallSiteRelatedAttrs() const { return llvm::DINode::FlagAllCallsDescribed; } + +llvm::DIExpression * +CGDebugInfo::createConstantValueExpression(clang::ValueDecl const *VD, + const APValue ) { + llvm::DIExpression *ValExpr = nullptr; felipepiovezan wrote: I know this is how the original code was written, but since we are making a separate function we are now in a position where we can delete this line, early-return on the `if` and replace all assignments to `ValExpr` with return statements. https://github.com/llvm/llvm-project/pull/70674 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][DebugInfo][NFC] Add createConstantValueExpression helper (PR #70674)
https://github.com/felipepiovezan approved this pull request. LGTM but left a couple of comments. Thanks for breaking this into two separate PRs. https://github.com/llvm/llvm-project/pull/70674 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][DebugInfo][NFC] Add createConstantValueExpression helper (PR #70674)
https://github.com/felipepiovezan edited https://github.com/llvm/llvm-project/pull/70674 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [IPSCCP] Variable not visible at Og. (PR #66745)
@@ -106,14 +107,71 @@ static void findReturnsToZap(Function , } } -static bool runIPSCCP( -Module , const DataLayout , FunctionAnalysisManager *FAM, -std::function GetTLI, -std::function GetTTI, -std::function GetAC, -std::function GetDT, -std::function GetBFI, -bool IsFuncSpecEnabled) { +static void createDebugConstantExpression(Module , GlobalVariable *GV) { + SmallVector GVEs; + GV->getDebugInfo(GVEs); + if (GVEs.size() != 1) +return; + + DIBuilder DIB(M); + + // Create integer constant expression. + auto createIntegerExpression = [](const Constant *CV) -> DIExpression * { +const APInt = cast(CV)->getValue(); +std::optional InitIntOpt; +if (API.isNonNegative()) + InitIntOpt = API.tryZExtValue(); +else if (auto Temp = API.trySExtValue()) + // Transform a signed optional to unsigned optional. + InitIntOpt = static_cast(*Temp); felipepiovezan wrote: Looking at this again, isn't this the equivalent of always doing a sign extension? We are basically saying: if this starts with a zero, add zeros; if this starts with a one, add ones. Unless I am missing something, this code could be: ``` if (std::optional InitIntOpt = API.trySExtValue()) return DIB.createConstantValueExpression(static_cast(*InitIntOpt)) return nullptr; ``` https://github.com/llvm/llvm-project/pull/66745 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [IPSCCP] Variable not visible at Og. (PR #66745)
@@ -106,14 +107,71 @@ static void findReturnsToZap(Function , } } -static bool runIPSCCP( -Module , const DataLayout , FunctionAnalysisManager *FAM, -std::function GetTLI, -std::function GetTTI, -std::function GetAC, -std::function GetDT, -std::function GetBFI, -bool IsFuncSpecEnabled) { +static void createDebugConstantExpression(Module , GlobalVariable *GV) { + SmallVector GVEs; + GV->getDebugInfo(GVEs); + if (GVEs.size() != 1) +return; + + DIBuilder DIB(M); + + // Create integer constant expression. + auto createIntegerExpression = [](const Constant *CV) -> DIExpression * { +const APInt = cast(CV)->getValue(); +std::optional InitIntOpt; +if (API.isNonNegative()) + InitIntOpt = API.tryZExtValue(); +else if (auto Temp = API.trySExtValue()) + // Transform a signed optional to unsigned optional. + InitIntOpt = static_cast(*Temp); felipepiovezan wrote: Looking at this again, isn't this the equivalent of always doing a sign extension? We are basically saying: if this starts with a zero, add zeros; if this starts with a one, add ones. Unless I am missing something, this code could be: ``` if (std::optional InitIntOpt = API.trySExtValue()) return DIB.createConstantValueExpression(static_cast(*InitIntOpt)) return nullptr; ``` https://github.com/llvm/llvm-project/pull/66745 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [IPSCCP] Variable not visible at Og. (PR #66745)
@@ -106,14 +107,71 @@ static void findReturnsToZap(Function , } } -static bool runIPSCCP( -Module , const DataLayout , FunctionAnalysisManager *FAM, -std::function GetTLI, -std::function GetTTI, -std::function GetAC, -std::function GetDT, -std::function GetBFI, -bool IsFuncSpecEnabled) { +static void createDebugConstantExpression(Module , GlobalVariable *GV) { + SmallVector GVEs; + GV->getDebugInfo(GVEs); + if (GVEs.size() != 1) +return; + + DIBuilder DIB(M); + + // Create integer constant expression. + auto createIntegerExpression = [](const Constant *CV) -> DIExpression * { +const APInt = cast(CV)->getValue(); +std::optional InitIntOpt; +if (API.isNonNegative()) + InitIntOpt = API.tryZExtValue(); +else if (auto Temp = API.trySExtValue()) + // Transform a signed optional to unsigned optional. + InitIntOpt = static_cast(*Temp); +return InitIntOpt ? DIB.createConstantValueExpression(InitIntOpt.value()) + : nullptr; + }; + + const Constant *CV = GV->getInitializer(); + Type *Ty = GV->getValueType(); + if (Ty->isIntegerTy()) { +DIExpression *InitExpr = createIntegerExpression(CV); +if (InitExpr) + GVEs[0]->replaceOperandWith(1, InitExpr); +return; + } + + if (Ty->isFloatTy() || Ty->isDoubleTy()) { +const APFloat = cast(CV)->getValueAPF(); +GVEs[0]->replaceOperandWith(1, DIB.createConstantValueExpression( + APF.bitcastToAPInt().getZExtValue())); +return; + } + + if (!Ty->isPointerTy()) +return; + + if (isa(CV)) { +GVEs[0]->replaceOperandWith(1, DIB.createConstantValueExpression(0)); +return; + } + if (const ConstantExpr *CE = dyn_cast(CV); + CE->getNumOperands() == 1) { +const Value *V = CE->getOperand(0); +const Constant *CV = dyn_cast(V); +if (CV && !isa(CV); felipepiovezan wrote: Yeah, I believe the intent here would be: ``` if (auto CI = dyn_cast_or_null(V)) { ... } ``` Note that `!isa(CV)` is not necessary: a global value is always a pointer, therefore it cannot be a `ConstantInt` https://github.com/llvm/llvm-project/pull/66745 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [IPSCCP] Variable not visible at Og. (PR #66745)
@@ -106,14 +107,71 @@ static void findReturnsToZap(Function , } } -static bool runIPSCCP( -Module , const DataLayout , FunctionAnalysisManager *FAM, -std::function GetTLI, -std::function GetTTI, -std::function GetAC, -std::function GetDT, -std::function GetBFI, -bool IsFuncSpecEnabled) { +static void createDebugConstantExpression(Module , GlobalVariable *GV) { + SmallVector GVEs; + GV->getDebugInfo(GVEs); + if (GVEs.size() != 1) +return; + + DIBuilder DIB(M); + + // Create integer constant expression. + auto createIntegerExpression = [](const Constant *CV) -> DIExpression * { +const APInt = cast(CV)->getValue(); +std::optional InitIntOpt; +if (API.isNonNegative()) + InitIntOpt = API.tryZExtValue(); +else if (auto Temp = API.trySExtValue()) + // Transform a signed optional to unsigned optional. + InitIntOpt = static_cast(*Temp); +return InitIntOpt ? DIB.createConstantValueExpression(InitIntOpt.value()) + : nullptr; + }; + + const Constant *CV = GV->getInitializer(); + Type *Ty = GV->getValueType(); + if (Ty->isIntegerTy()) { +DIExpression *InitExpr = createIntegerExpression(CV); +if (InitExpr) + GVEs[0]->replaceOperandWith(1, InitExpr); +return; + } + + if (Ty->isFloatTy() || Ty->isDoubleTy()) { +const APFloat = cast(CV)->getValueAPF(); +GVEs[0]->replaceOperandWith(1, DIB.createConstantValueExpression( + APF.bitcastToAPInt().getZExtValue())); +return; + } + + if (!Ty->isPointerTy()) +return; + + if (isa(CV)) { +GVEs[0]->replaceOperandWith(1, DIB.createConstantValueExpression(0)); +return; + } + if (const ConstantExpr *CE = dyn_cast(CV); + CE->getNumOperands() == 1) { +const Value *V = CE->getOperand(0); +const Constant *CV = dyn_cast(V); +if (CV && !isa(CV); felipepiovezan wrote: Yeah, I believe the intent here would be: ``` if (auto CI = dyn_cast_or_null(V)) { ... } ``` Note that `!isa(CV)` is not necessary: a global value is always a pointer, therefore it cannot be a `ConstantInt` https://github.com/llvm/llvm-project/pull/66745 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Propagate the DWARF version from the main compiler invocation to PCHC… (PR #66032)
https://github.com/felipepiovezan approved this pull request. I've confirmed that this fixes the `lang/objc/modules-objc-property/TestModulesObjCProperty.py` failure we see in the matrix bot for DWARF 5! https://github.com/llvm/llvm-project/pull/66032 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] d820772 - Revert "[Serialization] Place command line defines in the correct file"
Author: Felipe de Azevedo Piovezan Date: 2023-03-24T14:49:47-04:00 New Revision: d820772ce17215c3e2c0f72d16b2d3f14a8745fb URL: https://github.com/llvm/llvm-project/commit/d820772ce17215c3e2c0f72d16b2d3f14a8745fb DIFF: https://github.com/llvm/llvm-project/commit/d820772ce17215c3e2c0f72d16b2d3f14a8745fb.diff LOG: Revert "[Serialization] Place command line defines in the correct file" This reverts commit 72073fc95cd4793a853925ddc8cc3fb2118808a5. Added: Modified: clang-tools-extra/clangd/index/SymbolCollector.cpp clang/docs/ReleaseNotes.rst clang/lib/Serialization/ASTReader.cpp clang/lib/Serialization/ASTWriter.cpp clang/test/PCH/ms-pch-macro.c Removed: clang/test/PCH/macro-cmdline.c diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index 519aceec15a18..3179810b1b185 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -687,10 +687,8 @@ bool SymbolCollector::handleMacroOccurrence(const IdentifierInfo *Name, const auto = PP->getSourceManager(); auto DefLoc = MI->getDefinitionLoc(); - // Also avoid storing macros that aren't defined in any file, i.e. predefined - // macros like __DBL_MIN__ and those defined on the command line. + // Also avoid storing predefined macros like __DBL_MIN__. if (SM.isWrittenInBuiltinFile(DefLoc) || - SM.isWrittenInCommandLineFile(DefLoc) || Name->getName() == "__GCC_HAVE_DWARF2_CFI_ASM") return true; diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 526ea11303708..1f18de92b920c 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -177,8 +177,8 @@ Improvements to Clang's diagnostics - Diagnostic notes and fix-its are now generated for ``ifunc``/``alias`` attributes which point to functions whose names are mangled. - Diagnostics relating to macros on the command line of a preprocessed assembly - file or precompiled header are now reported as coming from the file - instead of . + file are now reported as coming from the file instead of + . - Clang constexpr evaluator now provides a more concise diagnostic when calling function pointer that is known to be null. - Clang now avoids duplicate warnings on unreachable ``[[fallthrough]];`` statements diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 6654df40010cb..0273fa1b839a5 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -654,10 +654,6 @@ static bool checkPreprocessorOptions( SmallVector ExistingMacroNames; collectMacroDefinitions(ExistingPPOpts, ExistingMacros, ); - // Use a line marker to enter the file, as the defines and - // undefines here will have come from the command line. - SuggestedPredefines += "# 1 \"\" 1\n"; - for (unsigned I = 0, N = ExistingMacroNames.size(); I != N; ++I) { // Dig out the macro definition in the existing preprocessor options. StringRef MacroName = ExistingMacroNames[I]; @@ -717,10 +713,6 @@ static bool checkPreprocessorOptions( } return true; } - - // Leave the file and return to . - SuggestedPredefines += "# 1 \"\" 2\n"; - if (Validation == OptionValidateStrictMatches) { // If strict matches are requested, don't tolerate any extra defines in // the AST file that are missing on the command line. @@ -1587,13 +1579,8 @@ bool ASTReader::ReadSLocEntry(int ID) { auto Buffer = ReadBuffer(SLocEntryCursor, Name); if (!Buffer) return true; -FileID FID = SourceMgr.createFileID(std::move(Buffer), FileCharacter, ID, -BaseOffset + Offset, IncludeLoc); -if (Record[3]) { - auto = - const_cast(SourceMgr.getSLocEntry(FID).getFile()); - FileInfo.setHasLineDirectives(); -} +SourceMgr.createFileID(std::move(Buffer), FileCharacter, ID, + BaseOffset + Offset, IncludeLoc); break; } diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 31e44b52929f4..d89ee4e6199fc 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -,11 +,6 @@ void ASTWriter::AddString(StringRef Str, RecordDataImpl ) { bool ASTWriter::PreparePathForOutput(SmallVectorImpl ) { assert(Context && "should have context when outputting path"); - // Leave special file names as they are. - StringRef PathStr(Path.data(), Path.size()); - if (PathStr == "" || PathStr == "") -return false; - bool Changed = cleanPathForOutput(Context->getSourceManager().getFileManager(), Path); diff --git a/clang/test/PCH/macro-cmdline.c b/clang/test/PCH/macro-cmdline.c deleted file mode 100644 index
[clang] 997dc7e - [debug-info][codegen] Prevent creation of self-referential SP node
Author: Felipe de Azevedo Piovezan Date: 2023-02-20T14:22:49-05:00 New Revision: 997dc7e00f49663b60a78e18df1dfecdf62a4172 URL: https://github.com/llvm/llvm-project/commit/997dc7e00f49663b60a78e18df1dfecdf62a4172 DIFF: https://github.com/llvm/llvm-project/commit/997dc7e00f49663b60a78e18df1dfecdf62a4172.diff LOG: [debug-info][codegen] Prevent creation of self-referential SP node The function `CGDebugInfo::EmitFunctionDecl` is supposed to create a declaration -- never a _definition_ -- of a subprogram. This is made evident by the fact that the SPFlags never have the "Declaration" bit set by that function. However, when `EmitFunctionDecl` calls `DIBuilder::createFunction`, it still tries to fill the "Declaration" argument by passing it the result of `getFunctionDeclaration(D)`. This will query an internal cache of previously created declarations and, for most code paths, we return nullptr; all is good. However, as reported in [0], there are pathological cases in which we attempt to recreate a declaration, so the cache query succeeds, resulting in a subprogram declaration whose declaration field points to another declaration. Through a series of RAUWs, the declaration field ends up pointing to the SP itself. Self-referential MDNodes can't be `unique`, which causes the verifier to fail (declarations must be `unique`). We can argue that the caller should check the cache first, but this is not a correctness issue (declarations are `unique` anyway). The bug is that `CGDebugInfo::EmitFunctionDecl` should always pass `nullptr` to the declaration argument of `DIBuilder::createFunction`, expressing the fact that declarations don't point to other declarations. AFAICT this is not something for which any reasonable meaning exists. This seems a lot like a copy-paste mistake that has survived for ~10 years, since other places in this file have the exact same call almost token-by-token. I've tested this by compiling LLVMSupport with and without the patch, O2 and O0, and comparing the dwarfdump of the lib. The dumps are identical modulo the attributes decl_file/producer/comp_dir. [0]: https://github.com/llvm/llvm-project/issues/59241 Differential Revision: https://reviews.llvm.org/D143921 Added: llvm/test/Verifier/disubprogram_declaration.ll Modified: clang/lib/CodeGen/CGDebugInfo.cpp llvm/docs/LangRef.rst llvm/lib/IR/Verifier.cpp Removed: diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index bb91b5116d71b..4dab595ada76b 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -4225,10 +4225,9 @@ void CGDebugInfo::EmitFunctionDecl(GlobalDecl GD, SourceLocation Loc, llvm::DINodeArray Annotations = CollectBTFDeclTagAnnotations(D); llvm::DISubroutineType *STy = getOrCreateFunctionType(D, FnType, Unit); - llvm::DISubprogram *SP = - DBuilder.createFunction(FDContext, Name, LinkageName, Unit, LineNo, STy, - ScopeLine, Flags, SPFlags, TParamsArray.get(), - getFunctionDeclaration(D), nullptr, Annotations); + llvm::DISubprogram *SP = DBuilder.createFunction( + FDContext, Name, LinkageName, Unit, LineNo, STy, ScopeLine, Flags, + SPFlags, TParamsArray.get(), nullptr, nullptr, Annotations); // Preserve btf_decl_tag attributes for parameters of extern functions // for BPF target. The parameters created in this loop are attached as diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index 4052e58faa7fb..b4c9cff4821cc 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -5776,11 +5776,12 @@ retained, even if their IR counterparts are optimized out of the IR. The .. _DISubprogramDeclaration: -When ``isDefinition: false``, subprograms describe a declaration in the type -tree as opposed to a definition of a function. If the scope is a composite -type with an ODR ``identifier:`` and that does not set ``flags: DIFwdDecl``, -then the subprogram declaration is uniqued based only on its ``linkageName:`` -and ``scope:``. +When ``spFlags: DISPFlagDefinition`` is not present, subprograms describe a +declaration in the type tree as opposed to a definition of a function. In this +case, the ``declaration`` field must be empty. If the scope is a composite type +with an ODR ``identifier:`` and that does not set ``flags: DIFwdDecl``, then +the subprogram declaration is uniqued based only on its ``linkageName:`` and +``scope:``. .. code-block:: text @@ -5789,9 +5790,9 @@ and ``scope:``. } !0 = distinct !DISubprogram(name: "foo", linkageName: "_Zfoov", scope: !1, -file: !2, line: 7, type: !3, isLocal: true, -isDefinition: true, scopeLine: 8, -containingType: !4, +file: !2, line: 7, type: !3, +
[clang] f84d30e - Reland "[codegen] Store address of indirect arguments on the stack"
Author: Felipe de Azevedo Piovezan Date: 2023-02-01T13:07:47-05:00 New Revision: f84d30e172b1f2365b1437b931d99156a76ed8f2 URL: https://github.com/llvm/llvm-project/commit/f84d30e172b1f2365b1437b931d99156a76ed8f2 DIFF: https://github.com/llvm/llvm-project/commit/f84d30e172b1f2365b1437b931d99156a76ed8f2.diff LOG: Reland "[codegen] Store address of indirect arguments on the stack" The commit was reverted due to a regression in debug information of an optimized code test in lldb. This has since been addressed by: 1. rGf753e5be8239: [LiveDebugValues] Allow EntryValue with OP_deref expressions 2. rG055f2f04e658: [mem2reg][debuginfo] Handle op_deref when converting dbg.declare Differential Revision: https://reviews.llvm.org/D141381 Added: Modified: clang/docs/ReleaseNotes.rst clang/lib/CodeGen/CGDebugInfo.cpp clang/lib/CodeGen/CGDebugInfo.h clang/lib/CodeGen/CGDecl.cpp clang/test/CodeGen/aarch64-ls64.c clang/test/CodeGen/atomic-arm64.c clang/test/CodeGenCXX/amdgcn-func-arg.cpp clang/test/CodeGenCXX/debug-info.cpp clang/test/CodeGenCXX/derived-to-base-conv.cpp clang/test/CodeGenCoroutines/coro-params-exp-namespace.cpp clang/test/CodeGenCoroutines/coro-params.cpp clang/test/OpenMP/for_firstprivate_codegen.cpp clang/test/OpenMP/parallel_firstprivate_codegen.cpp clang/test/OpenMP/sections_firstprivate_codegen.cpp clang/test/OpenMP/single_firstprivate_codegen.cpp clang/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp clang/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp clang/test/OpenMP/target_teams_distribute_parallel_for_simd_firstprivate_codegen.cpp clang/test/OpenMP/target_teams_distribute_simd_firstprivate_codegen.cpp clang/test/OpenMP/teams_distribute_firstprivate_codegen.cpp clang/test/OpenMP/teams_distribute_parallel_for_firstprivate_codegen.cpp clang/test/OpenMP/teams_distribute_parallel_for_simd_firstprivate_codegen.cpp clang/test/OpenMP/teams_distribute_simd_firstprivate_codegen.cpp clang/test/OpenMP/teams_firstprivate_codegen.cpp Removed: diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index e5d09ce61778e..9d9c5c6cd3f34 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -72,6 +72,9 @@ Improvements to Clang's diagnostics Non-comprehensive list of changes in this release - +- Clang now saves the address of ABI-indirect function parameters on the stack, + improving the debug information available in programs compiled without + optimizations. New Compiler Flags -- diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index fafc0f8015987..bb91b5116d71b 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -4840,9 +4840,10 @@ void CGDebugInfo::EmitDeclareOfBlockDeclRefVariable( llvm::DILocalVariable * CGDebugInfo::EmitDeclareOfArgVariable(const VarDecl *VD, llvm::Value *AI, - unsigned ArgNo, CGBuilderTy ) { + unsigned ArgNo, CGBuilderTy , + bool UsePointerValue) { assert(CGM.getCodeGenOpts().hasReducedDebugInfo()); - return EmitDeclare(VD, AI, ArgNo, Builder); + return EmitDeclare(VD, AI, ArgNo, Builder, UsePointerValue); } namespace { diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h index d807fb7fad576..a70b72e2d1420 100644 --- a/clang/lib/CodeGen/CGDebugInfo.h +++ b/clang/lib/CodeGen/CGDebugInfo.h @@ -489,10 +489,9 @@ class CGDebugInfo { /// Emit call to \c llvm.dbg.declare for an argument variable /// declaration. - llvm::DILocalVariable *EmitDeclareOfArgVariable(const VarDecl *Decl, - llvm::Value *AI, - unsigned ArgNo, - CGBuilderTy ); + llvm::DILocalVariable * + EmitDeclareOfArgVariable(const VarDecl *Decl, llvm::Value *AI, unsigned ArgNo, + CGBuilderTy , bool UsePointerValue = false); /// Emit call to \c llvm.dbg.declare for the block-literal argument /// to a block invocation function. diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp index ceaddc4e694ac..a70997f5b27b8 100644 --- a/clang/lib/CodeGen/CGDecl.cpp +++ b/clang/lib/CodeGen/CGDecl.cpp @@ -2476,6 +2476,8 @@ void CodeGenFunction::EmitParmDecl(const VarDecl , ParamValue Arg, Address AllocaPtr = Address::invalid(); bool DoStore = false; bool IsScalar = hasScalarEvaluationKind(Ty); + bool UseIndirectDebugAddress = false; + // If we already have a pointer to the argument, reuse the input pointer. if (Arg.isIndirect()) { // If we
Re: [clang] f2d301f - Revert "[codegen] Store address of indirect arguments on the stack"
Apologies, I’ll do it next time! In the meantime, the reason was posted in D141381 > On Jan 16, 2023, at 13:59, Roman Lebedev wrote: > > Reminder to please always mention the reason for the revert/recommit > in the commit message. > > On Mon, Jan 16, 2023 at 7:05 PM Felipe de Azevedo Piovezan via > cfe-commits wrote: >> >> >> Author: Felipe de Azevedo Piovezan >> Date: 2023-01-16T13:05:22-03:00 >> New Revision: f2d301fe82869f881b86b51da7b4752972c66707 >> >> URL: >> https://github.com/llvm/llvm-project/commit/f2d301fe82869f881b86b51da7b4752972c66707 >> DIFF: >> https://github.com/llvm/llvm-project/commit/f2d301fe82869f881b86b51da7b4752972c66707.diff >> >> LOG: Revert "[codegen] Store address of indirect arguments on the stack" >> >> This reverts commit 7e4447a17db4a070f01c8f8a87505a4b2a1b0e3a. >> >> Added: >> >> >> Modified: >>clang/docs/ReleaseNotes.rst >>clang/lib/CodeGen/CGDebugInfo.cpp >>clang/lib/CodeGen/CGDebugInfo.h >>clang/lib/CodeGen/CGDecl.cpp >>clang/test/CodeGen/aarch64-ls64.c >>clang/test/CodeGen/atomic-arm64.c >>clang/test/CodeGenCXX/amdgcn-func-arg.cpp >>clang/test/CodeGenCXX/debug-info.cpp >>clang/test/CodeGenCXX/derived-to-base-conv.cpp >>clang/test/CodeGenCoroutines/coro-params-exp-namespace.cpp >>clang/test/CodeGenCoroutines/coro-params.cpp >>clang/test/OpenMP/for_firstprivate_codegen.cpp >>clang/test/OpenMP/parallel_firstprivate_codegen.cpp >>clang/test/OpenMP/sections_firstprivate_codegen.cpp >>clang/test/OpenMP/single_firstprivate_codegen.cpp >>clang/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp >> >> clang/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp >> >> clang/test/OpenMP/target_teams_distribute_parallel_for_simd_firstprivate_codegen.cpp >>clang/test/OpenMP/target_teams_distribute_simd_firstprivate_codegen.cpp >>clang/test/OpenMP/teams_distribute_firstprivate_codegen.cpp >>clang/test/OpenMP/teams_distribute_parallel_for_firstprivate_codegen.cpp >> >> clang/test/OpenMP/teams_distribute_parallel_for_simd_firstprivate_codegen.cpp >>clang/test/OpenMP/teams_distribute_simd_firstprivate_codegen.cpp >>clang/test/OpenMP/teams_firstprivate_codegen.cpp >> >> Removed: >> >> >> >> >> diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst >> index 731d10ac64a3f..3bbab951e8a8c 100644 >> --- a/clang/docs/ReleaseNotes.rst >> +++ b/clang/docs/ReleaseNotes.rst >> @@ -500,9 +500,6 @@ Non-comprehensive list of changes in this release >> - Clang can now generate a PCH when using ``-fdelayed-template-parsing`` for >> code with templates containing loop hint pragmas, OpenMP pragmas, and >> ``#pragma unused``. >> -- Clang now saves the address of ABI-indirect function parameters on the >> stack, >> - improving the debug information available in programs compiled without >> - optimizations. >> >> >> New Compiler Flags >> >> diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp >> b/clang/lib/CodeGen/CGDebugInfo.cpp >> index f2d558456fde4..f53a9d00ae5fd 100644 >> --- a/clang/lib/CodeGen/CGDebugInfo.cpp >> +++ b/clang/lib/CodeGen/CGDebugInfo.cpp >> @@ -4822,10 +4822,9 @@ void CGDebugInfo::EmitDeclareOfBlockDeclRefVariable( >> >> llvm::DILocalVariable * >> CGDebugInfo::EmitDeclareOfArgVariable(const VarDecl *VD, llvm::Value *AI, >> - unsigned ArgNo, CGBuilderTy , >> - const bool UsePointerValue) { >> + unsigned ArgNo, CGBuilderTy ) >> { >> assert(CGM.getCodeGenOpts().hasReducedDebugInfo()); >> - return EmitDeclare(VD, AI, ArgNo, Builder, UsePointerValue); >> + return EmitDeclare(VD, AI, ArgNo, Builder); >> } >> >> namespace { >> >> diff --git a/clang/lib/CodeGen/CGDebugInfo.h >> b/clang/lib/CodeGen/CGDebugInfo.h >> index 10660a2550b56..95484a060cd88 100644 >> --- a/clang/lib/CodeGen/CGDebugInfo.h >> +++ b/clang/lib/CodeGen/CGDebugInfo.h >> @@ -487,9 +487,10 @@ class CGDebugInfo { >> >> /// Emit call to \c llvm.dbg.declare for an argument variable >> /// declaration. >> - llvm::DILocalVariable * >> - EmitDeclareOfArgVariable(const VarDecl *Decl, llvm::Value *AI, unsigned >> ArgNo, >> -
[clang] f2d301f - Revert "[codegen] Store address of indirect arguments on the stack"
Author: Felipe de Azevedo Piovezan Date: 2023-01-16T13:05:22-03:00 New Revision: f2d301fe82869f881b86b51da7b4752972c66707 URL: https://github.com/llvm/llvm-project/commit/f2d301fe82869f881b86b51da7b4752972c66707 DIFF: https://github.com/llvm/llvm-project/commit/f2d301fe82869f881b86b51da7b4752972c66707.diff LOG: Revert "[codegen] Store address of indirect arguments on the stack" This reverts commit 7e4447a17db4a070f01c8f8a87505a4b2a1b0e3a. Added: Modified: clang/docs/ReleaseNotes.rst clang/lib/CodeGen/CGDebugInfo.cpp clang/lib/CodeGen/CGDebugInfo.h clang/lib/CodeGen/CGDecl.cpp clang/test/CodeGen/aarch64-ls64.c clang/test/CodeGen/atomic-arm64.c clang/test/CodeGenCXX/amdgcn-func-arg.cpp clang/test/CodeGenCXX/debug-info.cpp clang/test/CodeGenCXX/derived-to-base-conv.cpp clang/test/CodeGenCoroutines/coro-params-exp-namespace.cpp clang/test/CodeGenCoroutines/coro-params.cpp clang/test/OpenMP/for_firstprivate_codegen.cpp clang/test/OpenMP/parallel_firstprivate_codegen.cpp clang/test/OpenMP/sections_firstprivate_codegen.cpp clang/test/OpenMP/single_firstprivate_codegen.cpp clang/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp clang/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp clang/test/OpenMP/target_teams_distribute_parallel_for_simd_firstprivate_codegen.cpp clang/test/OpenMP/target_teams_distribute_simd_firstprivate_codegen.cpp clang/test/OpenMP/teams_distribute_firstprivate_codegen.cpp clang/test/OpenMP/teams_distribute_parallel_for_firstprivate_codegen.cpp clang/test/OpenMP/teams_distribute_parallel_for_simd_firstprivate_codegen.cpp clang/test/OpenMP/teams_distribute_simd_firstprivate_codegen.cpp clang/test/OpenMP/teams_firstprivate_codegen.cpp Removed: diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 731d10ac64a3f..3bbab951e8a8c 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -500,9 +500,6 @@ Non-comprehensive list of changes in this release - Clang can now generate a PCH when using ``-fdelayed-template-parsing`` for code with templates containing loop hint pragmas, OpenMP pragmas, and ``#pragma unused``. -- Clang now saves the address of ABI-indirect function parameters on the stack, - improving the debug information available in programs compiled without - optimizations. New Compiler Flags diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index f2d558456fde4..f53a9d00ae5fd 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -4822,10 +4822,9 @@ void CGDebugInfo::EmitDeclareOfBlockDeclRefVariable( llvm::DILocalVariable * CGDebugInfo::EmitDeclareOfArgVariable(const VarDecl *VD, llvm::Value *AI, - unsigned ArgNo, CGBuilderTy , - const bool UsePointerValue) { + unsigned ArgNo, CGBuilderTy ) { assert(CGM.getCodeGenOpts().hasReducedDebugInfo()); - return EmitDeclare(VD, AI, ArgNo, Builder, UsePointerValue); + return EmitDeclare(VD, AI, ArgNo, Builder); } namespace { diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h index 10660a2550b56..95484a060cd88 100644 --- a/clang/lib/CodeGen/CGDebugInfo.h +++ b/clang/lib/CodeGen/CGDebugInfo.h @@ -487,9 +487,10 @@ class CGDebugInfo { /// Emit call to \c llvm.dbg.declare for an argument variable /// declaration. - llvm::DILocalVariable * - EmitDeclareOfArgVariable(const VarDecl *Decl, llvm::Value *AI, unsigned ArgNo, - CGBuilderTy , bool UsePointerValue = false); + llvm::DILocalVariable *EmitDeclareOfArgVariable(const VarDecl *Decl, + llvm::Value *AI, + unsigned ArgNo, + CGBuilderTy ); /// Emit call to \c llvm.dbg.declare for the block-literal argument /// to a block invocation function. diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp index a70997f5b27b8..ceaddc4e694ac 100644 --- a/clang/lib/CodeGen/CGDecl.cpp +++ b/clang/lib/CodeGen/CGDecl.cpp @@ -2476,8 +2476,6 @@ void CodeGenFunction::EmitParmDecl(const VarDecl , ParamValue Arg, Address AllocaPtr = Address::invalid(); bool DoStore = false; bool IsScalar = hasScalarEvaluationKind(Ty); - bool UseIndirectDebugAddress = false; - // If we already have a pointer to the argument, reuse the input pointer. if (Arg.isIndirect()) { // If we have a prettier pointer type at this point, bitcast to that. @@ -2489,19 +2487,6 @@ void CodeGenFunction::EmitParmDecl(const VarDecl , ParamValue Arg, auto AllocaAS = CGM.getASTAllocaAddressSpace(); auto *V =
[clang] 7e4447a - [codegen] Store address of indirect arguments on the stack
Author: Felipe de Azevedo Piovezan Date: 2023-01-16T11:14:55-03:00 New Revision: 7e4447a17db4a070f01c8f8a87505a4b2a1b0e3a URL: https://github.com/llvm/llvm-project/commit/7e4447a17db4a070f01c8f8a87505a4b2a1b0e3a DIFF: https://github.com/llvm/llvm-project/commit/7e4447a17db4a070f01c8f8a87505a4b2a1b0e3a.diff LOG: [codegen] Store address of indirect arguments on the stack With codegen prior to this patch, truly indirect arguments -- i.e. those that are not `byval` -- can have their debug information lost even at O0. Because indirect arguments are passed by pointer, and this pointer is likely placed in a register as per the function call ABI, debug information is lost as soon as the register gets clobbered. This patch solves the issue by storing the address of the parameter on the stack, using a similar strategy employed when C++ references are passed. In other words, this patch changes codegen from: ``` define @foo(ptr %arg) { call void @llvm.dbg.declare(%arg, [...], metadata !DIExpression()) ``` To: ``` define @foo(ptr %arg) { %ptr_storage = alloca ptr store ptr %arg, ptr %ptr_storage call void @llvm.dbg.declare(%ptr_storage, [...], metadata !DIExpression(DW_OP_deref)) ``` Some common cases where this may happen with C or C++ function calls: 1. "Big enough" trivial structures passed by value under the ARM ABI. 2. Structures that are non-trivial for the purposes of call (as per the Itanium ABI) when passed by value. A few tests were matching the wrong alloca (matching against the new alloca, instead of the old one), so they were updated to either match both allocas or include a `,` right after the alloca type, to prevent matching against a pointer type. Differential Revision: https://reviews.llvm.org/D141381 Added: Modified: clang/docs/ReleaseNotes.rst clang/lib/CodeGen/CGDebugInfo.cpp clang/lib/CodeGen/CGDebugInfo.h clang/lib/CodeGen/CGDecl.cpp clang/test/CodeGen/aarch64-ls64.c clang/test/CodeGen/atomic-arm64.c clang/test/CodeGenCXX/amdgcn-func-arg.cpp clang/test/CodeGenCXX/debug-info.cpp clang/test/CodeGenCXX/derived-to-base-conv.cpp clang/test/CodeGenCoroutines/coro-params-exp-namespace.cpp clang/test/CodeGenCoroutines/coro-params.cpp clang/test/OpenMP/for_firstprivate_codegen.cpp clang/test/OpenMP/parallel_firstprivate_codegen.cpp clang/test/OpenMP/sections_firstprivate_codegen.cpp clang/test/OpenMP/single_firstprivate_codegen.cpp clang/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp clang/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp clang/test/OpenMP/target_teams_distribute_parallel_for_simd_firstprivate_codegen.cpp clang/test/OpenMP/target_teams_distribute_simd_firstprivate_codegen.cpp clang/test/OpenMP/teams_distribute_firstprivate_codegen.cpp clang/test/OpenMP/teams_distribute_parallel_for_firstprivate_codegen.cpp clang/test/OpenMP/teams_distribute_parallel_for_simd_firstprivate_codegen.cpp clang/test/OpenMP/teams_distribute_simd_firstprivate_codegen.cpp clang/test/OpenMP/teams_firstprivate_codegen.cpp Removed: diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 3bbab951e8a8c..731d10ac64a3f 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -500,6 +500,9 @@ Non-comprehensive list of changes in this release - Clang can now generate a PCH when using ``-fdelayed-template-parsing`` for code with templates containing loop hint pragmas, OpenMP pragmas, and ``#pragma unused``. +- Clang now saves the address of ABI-indirect function parameters on the stack, + improving the debug information available in programs compiled without + optimizations. New Compiler Flags diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index f53a9d00ae5fd..f2d558456fde4 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -4822,9 +4822,10 @@ void CGDebugInfo::EmitDeclareOfBlockDeclRefVariable( llvm::DILocalVariable * CGDebugInfo::EmitDeclareOfArgVariable(const VarDecl *VD, llvm::Value *AI, - unsigned ArgNo, CGBuilderTy ) { + unsigned ArgNo, CGBuilderTy , + const bool UsePointerValue) { assert(CGM.getCodeGenOpts().hasReducedDebugInfo()); - return EmitDeclare(VD, AI, ArgNo, Builder); + return EmitDeclare(VD, AI, ArgNo, Builder, UsePointerValue); } namespace { diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h index 95484a060cd88..10660a2550b56 100644 --- a/clang/lib/CodeGen/CGDebugInfo.h +++ b/clang/lib/CodeGen/CGDebugInfo.h @@ -487,10 +487,9 @@ class CGDebugInfo { /// Emit call to \c llvm.dbg.declare for an argument variable /// declaration. - llvm::DILocalVariable