- User Since
- Oct 8 2012, 9:19 AM (289 w, 3 d)
Hi there :)
Tue, Apr 24
Sorry, I'm a bit confused.
Mon, Apr 23
You can use attributes to force inlining or non-inlining to create a
Is there anything else in the "-w" namespace other than the literal "-w" so
FWIW I don't fundamentalyl object to also having something like -wtest.
Probably needs a better name though (unfortunately the double-negative gets
confusing... - like you want to describe the set of diagnostics that should
not be used in test code, so that as a group might be "-Wnon-test" but then
"-Wno-non-test" is pretty awkward) - probably worth chatting to Richard
Smith about that, I reckon.
Not really my area of expertise - just skimming the commit history for the file it looks like Quentin did a bunch of nearby work, so perhaps he can help?
Sounds like sort of "-gno-really-line-tables-only" (I kid, somewhat)
Fri, Apr 20
I'd say if we can't actually get it totally clean & enable the warning, maybe it's not worth doing all this? & you (& others in your situation) can contribute patches on an as-needed basis?
Thu, Apr 19
I've pushed this further - it builds now & has no fallback in libBasic to the old behavior/table.
+1 - I'd rather not have a special case for filter+reverse if we can help
it. If filter can be made bidi so it "just works" without reverse knowing
anything about it, that seems better.
Tue, Apr 17
ANyway, seems OK.
OK - so would "skipDebugInstructionsForward" ever need the end iterator?
since it should always come across a non-debug instruction (all valid BBs
have at least one non-debug instruction, the terminator - though I guess
you might call this function on an invalid basic block during
construction/intermediate state changes, etc).
Mon, Apr 16
So does this make the other change NFC (under this assertion)? (ie: if the assertion is true, is "MBB" and "*Header" equivalent/the same thing?)
I'm not sure this is a practical direction to pursue - though perhaps
Sun, Apr 15
Fix up a few comments & then it seems fine.
Fri, Apr 13
Thu, Apr 12
But I don't think we start emitting the CU as we are going, right? I
thought it was handled much like the TUs would be by this patch - built...
And then emitted once it was fully built. (Indeed because new attributes
can be added to DIEs, forward references to future DIEs can exist, etc - we
currently have no support for streaming dwarf emission - the entire DIE
tree is built in memory, then emitted)
This would regress memory usage significantly - see r260578 / D17118 for more details. (17% decrease in memory usage when this was implemented)
Wed, Apr 11
Great - thanks for sticking with it, sorry for my dodgy explanations!
I meant that lib/LTO/LLVMBuild.txt should include "ExecutionEngine" in its
required_libraries. I wouldn't expect any changes would be needed to
tools/lto/* or tools/llvm-lto/*.
That sort of surprises me - if that were the case, there would be no point
having LLVMBuild.txt files in any libraries, I would think?
I meant what happens if you remove the changes to tools/lto/LLVMBuild.txt
and tools/llvm-lto/LLVMBuild.txt and /only/ have a change to
lib/LTO/LLVMBuild.txt - is that sufficient to make the build work?
Finish functional prototype - this now does remove the dependency from Basic to the various other libraries.
Is it sufficient to put the dependency MCJIT in lib/LTO/LLVMBuild.txt (since that's where the dependency on is) & not in the tools that /use/ the LTO lib (I expect they should pickup the indirect dependency implicitly?)?
I realize it's non-trivial to keep these all in the same list - but I'd
rather not create a new list only to remove it again later if it can be
helped. What particular issues did you encounter when attempting to use a
Tue, Apr 10
0.6% increase to debug_line (assuming you checked & this didn't increase
anything else? Can't see how it could, but just checking - /maybe/ some
inlining/debug_ranges stuff?) seems acceptable to me.
Hah, wow - didn't know you could include LLVM headers in godbolt code...
that's super handy :D
Mon, Apr 9
You'd have to add the ExecutionEngine library as a dependency from the LTO
library - by modifying lib/LTO/LLVMBuild.txt - then it should link, I would
Do you have the stats on this change in terms of line table size increase?
FWIW - I had some thoughts on this a while back:
Looks like ObjectMemoryBuffer is part of ExecutionEngine, so having libLTO
dependent on ExecutionEngine seems basically correct for the dependencies
that are there already. Whether or not those dependencies should be there
is another question, I guess. (maybe ObjectMemoryBuffer should be moved
somewhere more common, etc - but I wouldn't worry about doing that unless
the LTO dependency breaks things)
Out of curiosity - what was the motivation for this change? I'd expect that
copy would be faster than repeated push_back (though maybe LLVM can
optimize it to match)
Fri, Apr 6
Switch this to a C++ style static_cast and go ahead. Thanks!
Thu, Apr 5
Probably best not to create another list on the DISubprogram - you might be
able to generalize the existing list to contain "anything that might be
Rather than adding another list (if/when it comes to that) - we might want
to generalize the existing list to contain any/all local entities.
Tue, Apr 3
This still doesn't /sound/ right to me - the triviality of a class doesn't
seem like it would correctly allow you to deduce the triviality of specific
special members? Because the the class would only be considered trivial if
all the special members are trivial, probably?
I'm still not sure this description/name matches the examples given in the
other review from Reid.
Makes sense to me - I haven't applied/tested the patch against some recent
commits to LLVM I've made that broke gold/binutils ld based builders but
didn't break for me locally (using lld), but assuming it catches those -
yeah, it'd be useful to me & I'd totally turn it on locally & suggest
anyone else using lld with LLVM to do so as well.
iterator or iterator-like thing. One option would be a callback with Error
when the iterator is retrieved - and that's called back for any error (&
the user wouldn't need to differentiate the different kinds of error - the
iterator would stop when it couldn't continue). Or an Iterator-like but
not-actually-iterator API that exposes the Error during iteration (but
that'd probably look a bit awkward & be something like pair<Error,
Optional<T>> or, yes, as suggested - a separate Error type to communicate
"fail and cotninue" separately from "fail and stop"... )
I was picturing the parser being the thing that exposes the iterator - so
it would be an internal detail & wouldn't really warrant an extra Error
type - but I haven't thought about it too hard & maybe that doesn't make
I'm still a bit undecided about new Error types here - I think exposing an
iterator (or iterator-like thing) rather than special changes to the offset
or an error kind the user checks to see whether to iterate might be better?
That's fair (re: cast over &*)
Mon, Apr 2
Does the proposed clang change (using 'isTrivial') match MSVC's behavior in
Yeah, +1 for verifying whether or not DIFlagTypePassByValue/Reference is
sufficient for this.
Fri, Mar 30
Comment might need to be reworded, but might not even need a comment - if
we build with the warning on by default anyone who attempts to remove it
will find out...
Thu, Mar 29
I'd probably guess that the best way to expose this is to use a higher
level construct than a raw offset exposed to the user of the API - some
kind of iterator should be exposed & then if parsing can't continue, it
doesn't continue. Rather than exposing the raw offset to the user & having
them have to handle passing it back in, or detecting when not to do so.
Fair - the minor 'duplication' seems fine, given it skips the work of mapping the llvm::Type to an EVT, etc...
Wed, Mar 28
Yeah, before committing to any new asm syntax, worth a conversation on
llvm-dev with the usual crew & hopefully we can rope in some folks from GCC
or whatnot. It might even be worth discussing on dwarf-discuss, even though
it's not technically a dwarf issue. (& if you're feeling up for it - if
we're having conversations about assembly syntax extensions anyway, there's
a couple of others that might be nice - the ability to specify a CU for
line table entries (LLVM can produce multiple line tables with its
integrated assembler, for use with LTO and ThinLTO, but falls back to
producing a single line table when producing textual assembly) - and maybe
one day the address pool stuff we've talked about)
Mar 27 2018
Seems good to me
You mention a missed case in the description - where the target of a using decl is used and that causes the unused-using not to warn about the using decl? That seems like a big limitation. Does that mean the 'used' bit is being tracked in the wrong place, on the target of the using decl instead of on the using decl itself?
Mar 26 2018
Historically Clang's policy on warnings was, I think, much more
conservative than it seems to be today. There was a strong desire not to
implement off-by-default warnings, and to have warnings with an
exceptionally low false-positive rate - maybe the user-defined operator
detection was either assumed to, or demonstrated to, have a sufficiently
high false positive rate to not meet that high bar.