- User Since
- Mar 2 2013, 8:12 AM (393 w, 5 d)
Tue, Sep 15
I'm expecting this to cause all sorts of non-clang frontends to trigger this assertion, since we don't require types to have sizes. In fact, I have an open bug for the Swift frontend to try removing the static sizes from types, since LLDB prefers the dynamic runtime size anyway.
If we want to make this a requirement, we should probably implement it as a verifier check instead of an assertion. But I'm not sure if we want to require this for all frontends.
Sat, Sep 12
This looks like a good idea — people will probably have some options on the names and descriptions of the statistics :-)
Fri, Sep 11
Is this causing real problems? Usually we emit newer attributes even for older DWARF versions because most consumers just ignore unknown attributes.
Could you add test that showcases the effect?
I think this looks mostly good.
Wed, Sep 2
Tue, Sep 1
Mon, Aug 31
Fri, Aug 28
Assuming that it works, this seems fine.
Ignore line 0.
Yes, triggers a similar error in debug mode:
... o -MF lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o.d -o lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o -c /Volumes/Data/llvm-project/llvm/lib/Demangle/ItaniumDemangle.cpp Occurence of source location from an operand does not dominate instruction. br label %cond.end26, !dbg !2571 !2571 = !DILocation(line: 355, column: 10, scope: !2491) %cond = phi i8* [ %21, %cond.true24 ], [ null, %cond.false25 ], !dbg !2571
Wed, Aug 26
Sorry, I missed this!
Could you please also include a testcase?
What a function :-)
Tue, Aug 25
I should also mention that we are actively looking into ways to upstream the swift-lldb code into llvm.org. Now that both branches are licensed under Apache the only challenge left is to make the Swift support a real language *plugin* that can be conditionally compiled.
The largest downstream LLDB fork that I know about is swift-lldb, which is also open source and thus easy to check. There also exists a Rust version, and many custom forks for various specialized hardware. Putting patches up for review here gives the maintainers a chance to voice their concerns. For larger architectural changes we usually have a thread on lldb-dev to make sure we captured everyone's attention. We're under no obligation to not break anyone's private fork, but if there is an easy/reasonable way to make everyone happy we usually tend to choose that path.
Sometimes these odd leaf APIs are used by custom downstream variants of LLDB, but without any comment pointing into that direction and no unit test, I think this makes sense to remove. Also, we can easily add it back in later.
Mon, Aug 24
Fri, Aug 21
@shafik would you mind preparing a separate commit that add the REQUIRES everywhere it is needed, or if there are many, create an x86 subdir with an implicit requirement (like we do in the llvm/test/debuginfo tests)?
If it weren't for Rosetta, these would break on Apple Silicon machines, for example.
LGTM with that caveat
This looks nice! I'm somewhat suspicious that the new test doesn't specifically test the union case of the old test, but I'm assuming that would still work and your simpler tests covers the same code?
I'm fine with whatever numbering makes your development the easiest. My only ask is that if we still have a gap after this, please document what nodes are expected to go there, so there 1) won't be any future conflicts and 2) we don't pointlessly reserve space.
Thu, Aug 20
The code looks fine, I think the test needs an extra REQUIRES.
Thanks, this looks good to me — I just have a few inline comments.
@vsk We really need a richer API for MBB::erase that takes extra arguments and forces you to think about this.
Wed, Aug 19
I just learned that we no longer compute SDKROOT in Makefile.rules, so this seems resonable.
Thanks! Having a test would be really important to document the expected functionality and prevent future regressions.
Every time we change something here, we tend to find new unexpected behavior in a very complex LTO build a couple months later ;-)
Oh, one important thing: Can you add a section to the .rst documentation with a MIR example that explains what a DBG_VALUE_REF is / what it's used for, etc?
Mechanically this looks fine — would be good for others to chime in whether we have consensus over doing this change though.
Aug 18 2020
(gdb) b main
Breakpoint 1 at 0x800005ec: file test.c, line 0.
Aug 17 2020
If the problem that you want to solve is that the prologue ends at line 0, then massaging earlier passes until the right result happens to come up in one testcase is not going to be very effective. Yes, it may fix the testcase, but that won't last long, and the patch is introducing questionable behavior that will end up causing other problems.
Aug 15 2020
Aug 13 2020
Aug 12 2020
LGTM modulo David's comment about the test.
I only added a few non-substantial inline comments.
LGTM + what @dblaikie said :-)