[PATCH] D159490: Fix warning in MSVC

fdeazeve added a comment. I think this may have broken builds: worktrees/src1/clang/lib/AST/DeclPrinter.cpp:293:48: error: function definition is not allowed here static bool mustPrintOnLeftSide(const Attr *A) { Probably missing a `}`? Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D159490: Fix warning in MSVC

fdeazeve added a comment. In D159490#4643199 , @erichkeane wrote: > Fixed in 6c0b9e357617 > . I > didn't verify ahead of time, but hopefully the bots will yell at ME this time >

[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

fdeazeve added a comment. Forgot to add the failure message: /usr/include/c++/v1/experimental/propagate_const:138:11: error: no template named 'remove_reference_t'; did you mean 'remove_reference'? typedef remove_reference_t())> element_type; Repository: rG LLVM Github Monorepo

[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

fdeazeve added a comment. I believe this patch might have broken LLDB bots https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/46269/ I've confirmed that d2d77e050b32 is good while e9ef45635b77

[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

fdeazeve added a comment. It seems like non-lldb bots are also failing, if it makes it easier to reproduce: https://green.lab.llvm.org/green/view/Clang/job/clang-stage1-RA/30837/consoleFull#-54352964249ba4694-19c4-4d7e-bec5-911270d8a58c Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

fdeazeve added a comment. In D116203#3740015 , @cjdb wrote: > In D116203#3739856 , @fdeazeve > wrote: > >> It seems like non-lldb bots are also failing, if it makes it easier to >> reproduce: >> >>

[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

fdeazeve added a comment. In D116203#3740843 , @cjdb wrote: > I think there's a forward fix available, but since I can't repro locally, I'm > going to need to push it to main and observe how the tools fare. I need to be AFK for a bit now, but I can

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

fdeazeve added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.h:494 + CGBuilderTy , + const bool UsePointerValue = false); FWIW I used a `const` bool here to match the style already present in

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

fdeazeve added a comment. In D141381#4040314 , @dblaikie wrote: > Ah, so the intent is that this causes these indirect args to be handled the > same way as other arguments at -O0 - placed in an alloca, eg: Yup, that's correct! > Though I guess it

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

fdeazeve updated this revision to Diff 487782. fdeazeve added a comment. Simplified condition spelling Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141381/new/ https://reviews.llvm.org/D141381 Files: clang/lib/CodeGen/CGDebugInfo.cpp

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

fdeazeve added a comment. In D141381#4040071 , @probinson wrote: > just make sure it's not conditional on -g. This makes sense, I'll update the patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

fdeazeve created this revision. Herald added a subscriber: kristof.beyls. Herald added a project: All. fdeazeve requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. With codegen prior to this patch, truly indirect arguments -- i.e. those that

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

fdeazeve updated this revision to Diff 487784. fdeazeve added a comment. Improved comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141381/new/ https://reviews.llvm.org/D141381 Files: clang/lib/CodeGen/CGDebugInfo.cpp

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

fdeazeve updated this revision to Diff 488611. fdeazeve added a comment. Address review comments: - Remove unncesarry `const` qualifier from function declaration. - Add an item in Clang's release notes. I surveyed Clang's release notes in all major releases from 12.0 and couldn't find anything

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

fdeazeve added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.h:494 + CGBuilderTy , + const bool UsePointerValue = false); dblaikie wrote: > fdeazeve wrote: > > FWIW I used a `const` bool here to

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

fdeazeve updated this revision to Diff 488233. fdeazeve added a comment. Herald added subscribers: kosarev, jvesely. Change codegen to no longer depend on -g Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141381/new/

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

fdeazeve added a comment. In D141381#4040639 , @probinson wrote: > To get data about the code size impact, people typically build some large > codebase with/without their patch and compare the .text sizes. It's common to > use clang itself for this

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

fdeazeve updated this revision to Diff 489502. fdeazeve added a comment. Rebased Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141381/new/ https://reviews.llvm.org/D141381 Files: clang/docs/ReleaseNotes.rst clang/lib/CodeGen/CGDebugInfo.cpp

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

fdeazeve added a comment. In hindsight, this should have been obvious. While SROA will not touch this: define @foo(ptr %arg) { call void @llvm.dbg.declare(%arg, [...], metadata !DIExpression()) It completely destroys the debug information provided by: define @foo(ptr %arg) {

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7e4447a17db4: [codegen] Store address of indirect arguments on the stack (authored by fdeazeve). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

fdeazeve added a comment. There is a real regression in `lldb/test/API/functionalities/param_entry_vals/basic_entry_values/` when this is compiled with O2 : __attribute__((noinline)) void func15(StructPassedViaPointerToTemporaryCopy S) It seems

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

fdeazeve added a comment. Windows failures happening in Flang's MLIR stage, likely unrelated to this Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141381/new/ https://reviews.llvm.org/D141381 ___

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

fdeazeve added a comment. The code in Mem2Reg (Local.cpp) is wrong in the presence of DIExprs, but by luck we conservatively don't do anything. The function below is roughly doing: "if this store covers the entire size of the `DIVar`, rewrite the dbg declare into a dbg value. This is only

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

fdeazeve added a comment. https://reviews.llvm.org/D142160 Should address the incorrect handling of Mem2Reg, this is not the full picture though, as at the MIR level we handle these two differently: DEBUG_VALUE !DIExpression () indirect// this what dbg.declare gets lowered to

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

fdeazeve added a comment. In D141381#4058327 , @jmorse wrote: > Curious -- that feels like the kind of thing we should be able to support, > but adding more "special handling" to SROA isn't great. Agreed! I'll have a look to see how far SROA is from

[PATCH] D144651: [Serialization] Place command line defines in the correct file

fdeazeve added a comment. > It looks like green.lab.llvm.org is down, so can't look at that log, but > doing a local build and test of lldb (on a macbook, as this test is only > enabled on macos) I don't see this failure, or any other tests failing with > this assertion. It should be back

[PATCH] D144651: [Serialization] Place command line defines in the correct file

fdeazeve added a comment. In D144651#4219340 , @aaron.ballman wrote: > In D144651#4217750 , @john.brawn > wrote: > >> Unfortunately I still can't reproduce this even using exactly the same cmake >> command as

[PATCH] D144651: [Serialization] Place command line defines in the correct file

fdeazeve added a comment. I think given Michael's investigation, we'll go ahead and revert. That said, some feedback on his suggestion for: > I wonder if we should just not add the root paths to filenames. Would allows us to make progress in restoring this patch Repository: rG LLVM Github

[PATCH] D143921: [debug-info][codegen] Prevent creation of self-referential SP node

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG997dc7e00f49: [debug-info][codegen] Prevent creation of self-referential SP node (authored by fdeazeve). Repository: rG LLVM Github Monorepo

[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang

fdeazeve added a comment. Thanks for working on this! The idea looks good to me, but I am not familiar with this part of the codebase, so I'll defer the review to others Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148266/new/

[PATCH] D144651: [Serialization] Place command line defines in the correct file

fdeazeve added a comment. I did some local testing, and this no longer seems to crash for me. Hopefully the bots will be fine too Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144651/new/ https://reviews.llvm.org/D144651

[PATCH] D144651: [Serialization] Place command line defines in the correct file

fdeazeve added a comment. I can check out a copy of this patch and help test it Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144651/new/ https://reviews.llvm.org/D144651 ___ cfe-commits mailing list

[PATCH] D144651: [Serialization] Place command line defines in the correct file

fdeazeve added a comment. According to git-bisect, this patch is causing the LLDB bots to crash. https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/52690/console (They were failed for other reasons before, which is why it took a while for this to be identified) Clang is crashing with:

[PATCH] D143921: [debug-info][codegen] Prevent creation of self-referential SP node

fdeazeve created this revision. Herald added subscribers: jdoerfert, hiraditya. Herald added a project: All. fdeazeve requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: llvm-commits, cfe-commits, sstefan1. Herald added projects: clang, LLVM. The

[PATCH] D143921: [debug-info][codegen] Prevent creation of self-referential SP node

fdeazeve added a comment. Any testing suggestions here? I can use what we have on GH (cpp -> codegen test), but I'm not sure if there's a finer grained test we could use. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143921/new/

[PATCH] D143921: [debug-info][codegen] Prevent creation of self-referential SP node

fdeazeve updated this revision to Diff 497321. fdeazeve added a comment. Added verifier test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143921/new/ https://reviews.llvm.org/D143921 Files: clang/lib/CodeGen/CGDebugInfo.cpp

[PATCH] D143921: [debug-info][codegen] Prevent creation of self-referential SP node

fdeazeve added a comment. In D143921#4124533 , @aprantl wrote: > In D143921#4123218 , @fdeazeve > wrote: > >> Any testing suggestions here? I can use what we have on GH (cpp -> codegen >> test), but I'm not

[PATCH] D143921: [debug-info][codegen] Prevent creation of self-referential SP node

fdeazeve updated this revision to Diff 497713. fdeazeve added a comment. Fixed clang format issue Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143921/new/ https://reviews.llvm.org/D143921 Files: clang/lib/CodeGen/CGDebugInfo.cpp

[PATCH] D144999: [Clang][MC][MachO]Only emits compact-unwind format for "canonical" personality symbols. For the rest, use DWARFs.

fdeazeve added a comment. I believe this is breaking the LLDB tests: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/56106/changes https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/buildTimeTrend Build 56105 was passing, and the only relevant change seems to come from this