Uploaded a patch that corrects a merge conflict.

@MaskRay: We have added support for Split DWARF. We would appreciate, if you can try again llvm-dva with the binaries you used in your test cases. Thanks.

Following suggestions from @echristo in relation with the tool name, it was decided to renamed it as llvm-dva.

Uploaded an updated patch:

Uploaded a correct patch that includes:

% fllvm-diva --print=scopes /tmp/Debug/bin/clang             

Logical View:
           {File} '/tmp/debug/bin/clang'

             {CompileUnit} 'cc1_main.cpp.dwo'

             {CompileUnit} 'cc1as_main.cpp.dwo'

             {CompileUnit} 'cc1gen_reproducer_main.cpp.dwo'

             {CompileUnit} 'driver.cpp.dwo'

Are 'instructions', 'symbols', and 'types' unavailable?

% fllvm-diva --print=symbols /tmp/Debug/bin/clang

Logical View:
% fllvm-diva --print=types /tmp/Debug/bin/clang                                                                                                               

Logical View:

Some documented options appear to be unavailable.

% fllvm-diva --sort=offset /tmp/Debug/bin/clang       
llvm-diva: Unknown command line argument '--sort=offset'.  Try: '/tmp/RelA/bin/llvm-diva --help'
llvm-diva: Did you mean '--report=offset'?

For CMake changes, it'd be good to test a -DBUILD_SHARED_LIBS=on configuration. Many unspecified dependency can be caught. It also pushes the author to think whether certain dependencies are needed.

FAILED: lib/libLLVMDebugInfoLogicalView.so.12git
...
ld.lld: error: undefined symbol: llvm::raw_ostream::write(char const*, unsigned long)
ld.lld: error: undefined symbol: llvm::Twine::str[abi:cxx11]() const

@MaskRay Thanks very much for your valuable comments.

In D88661#2307121, @MaskRay wrote:

(If you use arc diff to upload a patch (https://llvm.org/docs/GettingStarted.html#sending-patches), reviewers can use arc patch D88661 to apply the patch locally.
This patch does not have a/ b/ prefixes (git format-patch or git diff) and does not apply cleanly)

This patch includes the a/ b/ prefixes.

In D78265#1999445, @vsk wrote:

Sorry about the breakage, I've renamed the variable in 210616bd38d589020b45f8cbbf9f9ef1296f2729.

In D78265#1999223, @chill wrote:

Looks like it's a scope error in gcc <= 5.5.
https://gcc.godbolt.org/z/ECqRJJ

The minimum required version to build LLVM GCC 5.1.
https://llvm.org/docs/GettingStarted.html#host-c-toolchain-both-compiler-and-standard-library

In D78265#1999052, @LukeGeeson wrote:

I cannot build llvm locally using ninja clean && ninja check-all with this patch due to the following error:

AArch64ConditionOptimizer.cpp: In member function 'llvm::MachineInstr* {anonymous}::AArch64ConditionOptimizer::findSuitableCompare(llvm::MachineBasicBlock*) ... no matching function call to 'prev(llvm::MachineInstr&)'

Using the following cmake command in llvm-project/build

cmake -G Ninja -DLLVM_ENABLE_PROJECTS="clang;lld" -DCMAKE_BUILD_TYPE=Debug -DLLVM_ENABLE_ASSERTIONS=On -DLLVM_TARGETS_TO_BUILD="ARM;X86;AArch64" -DLLVM_ENABLE_SOLVER_Z3=Off -DPARALLEL_LINK_JOBS=2 -DLLVM_USE_LINKER=gold ../llvm

It looks as though harbourmaster is failing as well.
Based on commit hash 3a53806 on master

In D77184#1960473, @thakis wrote:

I pushed Andrew's fix (thanks!) (with minor formatting tweaks) in dbb0d8ecb3a024bd6817ebd8ad8c5c199a51d933 . Let me know if you still see issues.

In D77184#1957632, @thakis wrote:

Thanks!

@aprantl, as per your feedback I have moved the test case to the correct location.

In D57444#1390350, @dstenb wrote:

Sorry for not noticing this before, but we noticed now that the test case is placed under test/CodeGen/X86/, although it is a simplifycfg test. Should it ideally be placed under test/Transforms/SimplifyCFG/?

Thanks @aprantl, @uabelho for your valuable review.

Updated patch to address comments from @aprantl .

Updated patch to remove the negative CHECK. Thanks for your feedback.

Thanks very much for your invaluable feedback and reviews.

Your change seems to cause build failures. The following is the error message:

@vsk: I have updated the patch to remove the test case pr39187-gline-tables-only.ll, as per your suggestion.

Thanks for your comments.

Thanks for your comments. I have updated the patch to address them.

Ping.

@courbet: Your commit seems to cause build issues:

@aprantl, @vsk: Thanks very much for your valuable comments and suggestions.

In D53287#1274290, @vsk wrote:

LGTM, thanks. I'm sorry for the delay in reviewing this.

@vsk: Do you have any additional comments? It is OK for you if I commit the patch?

Address issues raised by @aprantl.

This revision caused builbot failures.

The patch caused some assertions. For more details see PR39243.

@mstorsjo, The change has been reverted. Thanks for creating the bugzilla.

In D52887#1260298, @mstorsjo wrote:

This broke compilation for me, compiling some source files now fail due to triggered assertions, see PR39243 for details. I guess I'll revert this in a while unless there's someone to fix it soon.

Thanks very much to all the reviewers for your valuable comments and suggestions.

Address issues raised by the reviewers.

Address issues raised by the reviewers.

Address issues raised by the reviewers.

Thanks very much to all the reviewers for your valuable comments and suggestions.

Address issues raised by the reviewers.

In D52614#1248058, @dstenb wrote:

It would be good to change the title to reflect what is done in MCP (at least before submitting this).

Thanks very much to all the reviewers for your valuable comments and suggestions.

The general preference seems to land this patch as-is.

@aprantl and @probinson,

Address issues raised by the reviewers.

In D51976#1235508, @aprantl wrote:

Having read the real testcase now, I understand why you are dropping the debug info now. I would like to point out that we could do better. If we extended llvm.dbg.value to take more than one LLVM SSA Value, then we could produce a DIExpression that selects either %add or %sub depending on the value of %cmp. Let me know if you are interested in implementing this, it should be relatively straight forward to do and would come in useful in all sorts of other situations, too.

Address issues raised by the reviewers.

Address issues raised by the reviewers.

In D51976#1232030, @aprantl wrote:

It still isn't clear to me why you need to drop the debug info in the first place (instead of e.g., making it point to the select, or leave it on its operands) but I hope that that will become obvious once I see the actual testcase.

In D51583#1221878, @kristina wrote:

Maybe remove a comma in the comment? A lot of tools don't like trailing newlines.

Up to you, any NFC change is usually fine for this, though you didn't assign anyone as a reviewer.

In D50887#1216171, @vsk wrote:

Thanks, LGTM. If you don't have commit access yet [1], I'd be happy to commit this on your behalf.

[1] https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access

@vsk & @aprantl Thanks your reviews.

Update the original patch, as the results from the investigation to replace the algorithm used by 'collectDebugValues', showed some additional complexities.

In D50887#1212594, @vsk wrote:

@CarlosAlbertoEnciso thanks for investigating this and testing out a new way to collect debug values. Given the complexity of replacing the current algorithm, and that it's not strictly related to fixing MachineCSE, I'd recommend keeping the current algorithm (i.e not using MachineRegisterInfo for now). That will allow us to land a fix in tree soon and hopefully iterate on it.

In D50887#1212565, @aprantl wrote:

Do you have OrderMap readily available or would you need to compute it first?

In D50887#1207849, @vsk wrote:

This is looking really good. I think it's great that the test exercises more than just MachineCSE in isolation. Just two more minor comments (inline) --

Following feedback from the reviewers:

Is there anything I can add to this patch in order to submit it?
This patch is blocking the review https://reviews.llvm.org/D44826 which is already approved.

A minor modification to allow the comments for MarkNamespaceAliasReferenced to be picked up by doxygen.