Yes you have. Sorry for the noise.

In D73148#1835488, @labath wrote:

In D73148#1834955, @vsk wrote:

In D73148#1832897, @clayborg wrote:

Actually it would be nice to have a test that will trigger on at least one build bot that runs ASAN?

I'll add an end-to-end test for DW_OP_piece, though I worry it might be brittle.

I don't think that's a good way to write a test like this. I suggest checking out test/Shell/SymbolFile/DWARF/DW_OP_piece-struct.s for an dwarf expression test that does not rely on guessing the DW_OP opcodes used by the compiler. Maybe you can just take that test and tweak it to do what you need?

Nice.

In D72797#1834089, @djtodoro wrote:

Hmm.. it looks lika a bug.

@aprantl What do you think?

Hmm. I would argue that the tuple discussion makes less sense in the context of this particular rewriting (since it really deals with getting rid of extract_slices/insert_slices using other slice ops). Cleaning up tuple uses is a nice bonus, but not a requirement, since tuples are really a part of the vector dialect to start with, as the leaking example shows. If you want something that blocks any tuple uses, I would argue that we need another pattern population for that, and run that at places where we need to guarantee they are gone (such as the lowering to LLVM, although the "legality" part takes care of it there already).

Sorry, I take my acceptance back. I do not think this is required. If you rebase to HEAD of master, it should work without this change. I think, when you reverted the changes to LLVMLibCRules.cmake, you removed LLVMSupport from the list of link libraries for add_libc_unittest targets.

In D70121#1824298, @Orlando wrote:

Thanks @djtodoro for looking at this. Sorry for this really delayed response!

This was initially going to be a ping, but as a result of some light offline
discussion I have a few questions. For clarification an "overlap" here refers to
a partial overlap between fragments in DIExpressions.

In D72675#1839492, @wristow wrote:

1. Should we enable FMA "by default" at (for example) '-O2'?

Well my point here is that if you added the patterns to fold the tuples, you would do as good a job as you could in the absence of region/function boundaries.
Here you created an example that leaks in an unfixable way (because we have conciously decided to no lower the tuple<vector> type to LLVMIR and take the strong position that it must canonicalize/fold/DCE away).

Thanks for the fix.

ping

Here is a proposal: we add two children to -Wrange-loop-analysis.

In D71566#1839462, @aaron.ballman wrote:

Continues to LG, do you need me to commit on your behalf?

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

In D73028#1839572, @rsmith wrote:

In D73028#1839494, @steveire wrote:

In D73028#1839383, @rsmith wrote:

The follow-up is here: https://reviews.llvm.org/D73029 .

I have the need to change the ascending traversal implemented in ASTContext.cpp with a map (populated while descending) to make it skip nodes too during parent traversal.

I didn't want to have descending traversal in Expr.cpp and ascending traversal in ASTContext.cpp as they would be likely to go out of sync, so moving this implementation here allows extending the purpose in the follow-up.

The ascending traversal should not be in the AST library at all. If tooling or static analysis wants this, that's fine, but navigating upwards is not something the AST supports, and we do not want any part of clang proper developing a reliance on having a parent map, so we're working on moving the parent map out of ASTContext (with a plan to eventually move it out of the AST library). See D71313 for related work in this area.

The MBFIWrapper change seems NFC, can it be extracted out first?

lgtm

In D73380#1839603, @xbolva00 wrote:

LLVM already infers noalias nonnull for eg. _Znwm so noalias and nonnull info added by clang will not increase power of LLVM. Or?

To be honest, i don't really believe it is LLVM place to deduce attributes on standard library functions like these, frontend should be doing that.
Because as noted in patch description, if noalias is blindly "deduced" by LLVM,
that is a miscompile if -fno-assume-sane-operator-new was specificed.

In D72665#1839600, @rnk wrote:

From the commit message, this sounds like it is fixing a bug. Can it be tested? Did you have a test already, or did you forget to add it?

Otherwise, code looks good.

Also move the ashr/lshr and trunc patterns into freelyNegateValue() and keep existing handling regarding one-use checks.