[DebugInfo] Enforce implicit constraints on `distinct` MDNodes

[scott.linder]

Summary

Add UNIQUED and DISTINCT properties in Metadata.def and use them to
implement restrictions on the distinct property of MDNodes:

• DIExpression can currently be parsed from IR or read from bitcode as distinct, but this property is silently dropped when printing to IR. This causes accepted IR to fail to round-trip. As DIExpression appears inline at each use in the canonical form of IR, it cannot actually be distinct anyway, as there is no syntax to describe it.
• DICompileUnit is already restricted to always being distinct, but along with adding general support for the inverse restriction I went ahead and described this in Metadata.def and updated the parser to be general. Future nodes which have this restriction can share this support.

The new UNIQUED property applies to DIExpression, and forbids it to be
distinct. It also implies it are canonically printed inline at each
use, rather than via MDNode ID.

The new DISTINCT property applies to DICompileUnit, and requires it to
be distinct.

A potential alternative change is to forbid the non-inline syntax for
DIExpression entirely, as is done with DIArgList implicitly by requiring
it appear in the context of a function. For example, we would forbid:

!named = !{!0}
!0 = !DIExpression()

Instead we would only accept the equivalent inlined version:

!named = !{!DIExpression()}

This essentially removes the ability to create a distinct DIExpression
by construction, as there is no syntax for distinct inline. If this
patch is accepted as-is, the result would be that the non-canonical
version is accepted, but the following would be an error and produce a diagnostic:

!named = !{!0}
; error: 'distinct' not allowed for !DIExpression()
!0 = distinct !DIExpression()

Also update some documentation to consistently use the inline syntax for
DIExpression, and to describe the restrictions on distinct for nodes
where applicable.

[t-tye]

LGTM Thanks for documenting the differences.

[scott.linder]

Implement inline case in AssemblyWriter::printNamedMDNode based on UNIQUED-ness.

I'm not sure my overall approach to this patch is actually the best way to
proceed. The biggest issue is that DIArgList is more particular about where it
can appear, so most of this generic support for UNIQUED nodes is just dead code
for DIArgList. It still seems like having a lot of if (isa<Foo>) ...;
littered around the IR parser/printer, bitcode reader/writer, MIR
reader/writer, etc. to implement inline nodes is more error prone, and we still
need to fix the bug with distinct either way.

Does anyone have thoughts on what approach to take?

[StephenTozer]

Good patch, this is a good way of generifying the logic for inline metadata - and improving the documentation at the same time, which is a big plus!

In D104827#2839615, @scott.linder wrote:

I'm not sure my overall approach to this patch is actually the best way to
proceed. The biggest issue is that DIArgList is more particular about where it
can appear, so most of this generic support for UNIQUED nodes is just dead code
for DIArgList.

I think this is mostly unavoidable as-is; this patch does nicely take care of the inline printing/parsing logic, trying to also simplify the function-local aspects would be out of scope. In that regard, the class that DIArgList shares its behaviour with is ValueAsMetadata/LocalAsMetadata. Unfortunately there is no easy way to generify that logic, as ValueAsMetadata is a unique case since it is printed as i32 %value rather than !ClassName(...). And I don't think I'd quite call the generic support "dead code" for DIArgList - it may be an error in all cases except when parsed as a function operand, but it still does need to be handled, and if some change in future affects how we print/parse inline metadata then that change must still apply equally to both DIExpression and DIArgList, which this patch facilitates.

A potential alternative change is to forbid the non-inline syntax for
DIExpression entirely, as is done with DIArgList implicitly by requiring
it appear in the context of a function.

Personally, I could go either way with this. Currently, DIExpression does not actually use function state in any way, but will still only ever be used in the context of a function. In particular, I'm not sure what value there could ever be in using a DIExpression in a named metadata node. It seems that in principle it would be better suited to being explicitly function-local metadata, but I'm not sure if it's worth putting in the effort to make the change when doing so doesn't grant us much benefit and also breaks parsing for old IR that contains ID'd DIExpressions.

[scott.linder]

Assert based on UNIQUED/DISTINCT-ness in ModuleBitcodeWriter

[uabelho]

Hi,

We've noticed that this patch changes things so bcfiles created with the patch cannot be parsed with binaries built before it.
E.g if I create foo.bc with a clang binary built from commit 8cd35ad854ab (this patch) with:

clang -save-temps foo.c -S -g

and then try to run opt built from aad87328fab (the commit before this patch) with

opt foo.bc -S > /dev/null

it results in

opt: ../lib/Bitcode/Reader/MetadataLoader.cpp:366: (anonymous namespace)::(anonymous namespace)::PlaceholderQueue::~PlaceholderQueue(): Assertion `empty() && "PlaceholderQueue hasn't been flushed before being destroyed"' failed.
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:
0.	Program arguments: build-all-builtins/bin/opt foo.bc -S
 #0 0x0000000002a11623 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (build-all-builtins/bin/opt+0x2a11623)
 #1 0x0000000002a0f2de llvm::sys::RunSignalHandlers() (build-all-builtins/bin/opt+0x2a0f2de)
 #2 0x0000000002a119a6 SignalHandler(int) Signals.cpp:0:0
 #3 0x00007f2c97079630 __restore_rt sigaction.c:0:0
 #4 0x00007f2c947ac387 raise (/lib64/libc.so.6+0x36387)
 #5 0x00007f2c947ada78 abort (/lib64/libc.so.6+0x37a78)
 #6 0x00007f2c947a51a6 __assert_fail_base (/lib64/libc.so.6+0x2f1a6)
 #7 0x00007f2c947a5252 (/lib64/libc.so.6+0x2f252)
 #8 0x0000000003398a64 llvm::MetadataLoader::MetadataLoaderImpl::parseMetadata(bool) (build-all-builtins/bin/opt+0x3398a64)
 #9 0x00000000033a19ac llvm::MetadataLoader::parseMetadata(bool) (build-all-builtins/bin/opt+0x33a19ac)
#10 0x00000000033730a8 (anonymous namespace)::BitcodeReader::parseFunctionBody(llvm::Function*) BitcodeReader.cpp:0:0
#11 0x0000000003370f7c (anonymous namespace)::BitcodeReader::materialize(llvm::GlobalValue*) BitcodeReader.cpp:0:0
#12 0x00000000033720aa (anonymous namespace)::BitcodeReader::materializeModule() BitcodeReader.cpp:0:0
#13 0x000000000221fc8a llvm::Module::materializeAll() (build-all-builtins/bin/opt+0x221fc8a)
#14 0x000000000336acb1 llvm::BitcodeModule::getModuleImpl(llvm::LLVMContext&, bool, bool, bool, llvm::function_ref<llvm::Optional<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (llvm::StringRef)>) (build-all-builtins/bin/opt+0x336acb1)
#15 0x000000000336ef60 llvm::parseBitcodeFile(llvm::MemoryBufferRef, llvm::LLVMContext&, llvm::function_ref<llvm::Optional<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (llvm::StringRef)>) (build-all-builtins/bin/opt+0x336ef60)
#16 0x00000000024521a0 llvm::parseIR(llvm::MemoryBufferRef, llvm::SMDiagnostic&, llvm::LLVMContext&, llvm::function_ref<llvm::Optional<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (llvm::StringRef)>) (build-all-builtins/bin/opt+0x24521a0)
#17 0x000000000245262e llvm::parseIRFile(llvm::StringRef, llvm::SMDiagnostic&, llvm::LLVMContext&, llvm::function_ref<llvm::Optional<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (llvm::StringRef)>) (build-all-builtins/bin/opt+0x245262e)
#18 0x0000000000782acb main (build-all-builtins/bin/opt+0x782acb)
#19 0x00007f2c94798555 __libc_start_main (/lib64/libc.so.6+0x22555)
#20 0x000000000076f800 _start (build-all-builtins/bin/opt+0x76f800)
Abort

I suppose this is not expected?

foo.bc2 KBDownload

[dexonsmith]

That looks bad — best to revert while that's investigated.

[JDevlieghere]

In D104827#2855970, @dexonsmith wrote:

That looks bad — best to revert while that's investigated.

This also breaks lang/cpp/member-and-local-vars-with-same-name/TestMembersAndLocalsWithSameName.py in the LLDB test suite:

Assertion failed: ((FromValue || !(isa<LocalAsMetadata>(MD) || isa<DIArgList>(MD))) && "Unexpected function-local metadata outside of value argument"), function WriteAsOperandInternal, file AsmWriter.cpp, line 2519.

Given that it's Friday, I'm going to go ahead and revert this to get our bot back to green.

[scott.linder]

In D104827#2854825, @uabelho wrote:

Hi,

We've noticed that this patch changes things so bcfiles created with the patch cannot be parsed with binaries built before it.
E.g if I create foo.bc with a clang binary built from commit 8cd35ad854ab (this patch) with:

clang -save-temps foo.c -S -g

and then try to run opt built from aad87328fab (the commit before this patch) with

opt foo.bc -S > /dev/null

it results in

opt: ../lib/Bitcode/Reader/MetadataLoader.cpp:366: (anonymous namespace)::(anonymous namespace)::PlaceholderQueue::~PlaceholderQueue(): Assertion `empty() && "PlaceholderQueue hasn't been flushed before being destroyed"' failed.
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:
0.	Program arguments: build-all-builtins/bin/opt foo.bc -S
 #0 0x0000000002a11623 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (build-all-builtins/bin/opt+0x2a11623)
 #1 0x0000000002a0f2de llvm::sys::RunSignalHandlers() (build-all-builtins/bin/opt+0x2a0f2de)
 #2 0x0000000002a119a6 SignalHandler(int) Signals.cpp:0:0
 #3 0x00007f2c97079630 __restore_rt sigaction.c:0:0
 #4 0x00007f2c947ac387 raise (/lib64/libc.so.6+0x36387)
 #5 0x00007f2c947ada78 abort (/lib64/libc.so.6+0x37a78)
 #6 0x00007f2c947a51a6 __assert_fail_base (/lib64/libc.so.6+0x2f1a6)
 #7 0x00007f2c947a5252 (/lib64/libc.so.6+0x2f252)
 #8 0x0000000003398a64 llvm::MetadataLoader::MetadataLoaderImpl::parseMetadata(bool) (build-all-builtins/bin/opt+0x3398a64)
 #9 0x00000000033a19ac llvm::MetadataLoader::parseMetadata(bool) (build-all-builtins/bin/opt+0x33a19ac)
#10 0x00000000033730a8 (anonymous namespace)::BitcodeReader::parseFunctionBody(llvm::Function*) BitcodeReader.cpp:0:0
#11 0x0000000003370f7c (anonymous namespace)::BitcodeReader::materialize(llvm::GlobalValue*) BitcodeReader.cpp:0:0
#12 0x00000000033720aa (anonymous namespace)::BitcodeReader::materializeModule() BitcodeReader.cpp:0:0
#13 0x000000000221fc8a llvm::Module::materializeAll() (build-all-builtins/bin/opt+0x221fc8a)
#14 0x000000000336acb1 llvm::BitcodeModule::getModuleImpl(llvm::LLVMContext&, bool, bool, bool, llvm::function_ref<llvm::Optional<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (llvm::StringRef)>) (build-all-builtins/bin/opt+0x336acb1)
#15 0x000000000336ef60 llvm::parseBitcodeFile(llvm::MemoryBufferRef, llvm::LLVMContext&, llvm::function_ref<llvm::Optional<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (llvm::StringRef)>) (build-all-builtins/bin/opt+0x336ef60)
#16 0x00000000024521a0 llvm::parseIR(llvm::MemoryBufferRef, llvm::SMDiagnostic&, llvm::LLVMContext&, llvm::function_ref<llvm::Optional<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (llvm::StringRef)>) (build-all-builtins/bin/opt+0x24521a0)
#17 0x000000000245262e llvm::parseIRFile(llvm::StringRef, llvm::SMDiagnostic&, llvm::LLVMContext&, llvm::function_ref<llvm::Optional<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (llvm::StringRef)>) (build-all-builtins/bin/opt+0x245262e)
#18 0x0000000000782acb main (build-all-builtins/bin/opt+0x782acb)
#19 0x00007f2c94798555 __libc_start_main (/lib64/libc.so.6+0x22555)
#20 0x000000000076f800 _start (build-all-builtins/bin/opt+0x76f800)
Abort

I suppose this is not expected?

foo.bc2 KBDownload

Thank you for letting me know, and for the reproducer! Curiously this is just on the return error(...) path in upgradeDIExpression, which I just didn't test (as I'm not sure how to in the current LLVM infrastructure). I assume this is because normally bitcode error returns occur after most of the bitcode file has already been read, so there are no pending placeholders, but I need to learn some more about the bitcode parser in general to be sure I understand the issue. My first thought is that we should just flush placeholders on an error, as we are about to just bail anyway.

In D104827#2856347, @JDevlieghere wrote:

In D104827#2855970, @dexonsmith wrote:

That looks bad — best to revert while that's investigated.

This also breaks lang/cpp/member-and-local-vars-with-same-name/TestMembersAndLocalsWithSameName.py in the LLDB test suite:

Assertion failed: ((FromValue || !(isa<LocalAsMetadata>(MD) || isa<DIArgList>(MD))) && "Unexpected function-local metadata outside of value argument"), function WriteAsOperandInternal, file AsmWriter.cpp, line 2519.

Given that it's Friday, I'm going to go ahead and revert this to get our bot back to green.

Thank you for reverting, I was out Friday+Monday. I thought I was safe to move the assert that is triggering here up further in the function body to make it stronger, but evidently I overlooked some cases. I'll identify them and either fix them elsewhere or restore the assert (possibly with a comment describing the rationale). Thanks again!

[scott.linder]

In D104827#2854825, @uabelho wrote:

Hi,

We've noticed that this patch changes things so bcfiles created with the patch cannot be parsed with binaries built before it.
E.g if I create foo.bc with a clang binary built from commit 8cd35ad854ab (this patch) with:

clang -save-temps foo.c -S -g

and then try to run opt built from aad87328fab (the commit before this patch) with

opt foo.bc -S > /dev/null

it results in

opt: ../lib/Bitcode/Reader/MetadataLoader.cpp:366: (anonymous namespace)::(anonymous namespace)::PlaceholderQueue::~PlaceholderQueue(): Assertion `empty() && "PlaceholderQueue hasn't been flushed before being destroyed"' failed.
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:
0.	Program arguments: build-all-builtins/bin/opt foo.bc -S
 #0 0x0000000002a11623 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (build-all-builtins/bin/opt+0x2a11623)
 #1 0x0000000002a0f2de llvm::sys::RunSignalHandlers() (build-all-builtins/bin/opt+0x2a0f2de)
 #2 0x0000000002a119a6 SignalHandler(int) Signals.cpp:0:0
 #3 0x00007f2c97079630 __restore_rt sigaction.c:0:0
 #4 0x00007f2c947ac387 raise (/lib64/libc.so.6+0x36387)
 #5 0x00007f2c947ada78 abort (/lib64/libc.so.6+0x37a78)
 #6 0x00007f2c947a51a6 __assert_fail_base (/lib64/libc.so.6+0x2f1a6)
 #7 0x00007f2c947a5252 (/lib64/libc.so.6+0x2f252)
 #8 0x0000000003398a64 llvm::MetadataLoader::MetadataLoaderImpl::parseMetadata(bool) (build-all-builtins/bin/opt+0x3398a64)
 #9 0x00000000033a19ac llvm::MetadataLoader::parseMetadata(bool) (build-all-builtins/bin/opt+0x33a19ac)
#10 0x00000000033730a8 (anonymous namespace)::BitcodeReader::parseFunctionBody(llvm::Function*) BitcodeReader.cpp:0:0
#11 0x0000000003370f7c (anonymous namespace)::BitcodeReader::materialize(llvm::GlobalValue*) BitcodeReader.cpp:0:0
#12 0x00000000033720aa (anonymous namespace)::BitcodeReader::materializeModule() BitcodeReader.cpp:0:0
#13 0x000000000221fc8a llvm::Module::materializeAll() (build-all-builtins/bin/opt+0x221fc8a)
#14 0x000000000336acb1 llvm::BitcodeModule::getModuleImpl(llvm::LLVMContext&, bool, bool, bool, llvm::function_ref<llvm::Optional<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (llvm::StringRef)>) (build-all-builtins/bin/opt+0x336acb1)
#15 0x000000000336ef60 llvm::parseBitcodeFile(llvm::MemoryBufferRef, llvm::LLVMContext&, llvm::function_ref<llvm::Optional<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (llvm::StringRef)>) (build-all-builtins/bin/opt+0x336ef60)
#16 0x00000000024521a0 llvm::parseIR(llvm::MemoryBufferRef, llvm::SMDiagnostic&, llvm::LLVMContext&, llvm::function_ref<llvm::Optional<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (llvm::StringRef)>) (build-all-builtins/bin/opt+0x24521a0)
#17 0x000000000245262e llvm::parseIRFile(llvm::StringRef, llvm::SMDiagnostic&, llvm::LLVMContext&, llvm::function_ref<llvm::Optional<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (llvm::StringRef)>) (build-all-builtins/bin/opt+0x245262e)
#18 0x0000000000782acb main (build-all-builtins/bin/opt+0x782acb)
#19 0x00007f2c94798555 __libc_start_main (/lib64/libc.so.6+0x22555)
#20 0x000000000076f800 _start (build-all-builtins/bin/opt+0x76f800)
Abort

I suppose this is not expected?

foo.bc2 KBDownload

This seems to be a long-standing bug with the bitcode reader, where every return error("Invalid record") in llvm::MetadataLoader::MetadataLoaderImpl::parseOneMetadata is untested code that actually just triggers an assert in PlaceholderQueue::~PlaceholderQueue. For example, the last version bump for DIExpression has the same issue:

user@host:~/llvm-project/main:main$ git switch --detach ffc498dfcc8d67e6e9acaebb690377f674457ab4
Updating files: 100% (101031/101031), done.
HEAD is now at ffc498dfcc8d Align definition of DW_OP_plus with DWARF spec [3/3]
user@host:~/llvm-project/main:(ffc498dfcc8d...)$ ninja -C release/ clang
ninja: Entering directory `release/'
...
[2569/2569] Creating executable symlink bin/clang
user@host:~/llvm-project/main:(ffc498dfcc8d...)$ echo 'void f(int x) {}' | release/bin/clang -x c - -c -emit-llvm -o- -g >
../f.bc
user@host:~/llvm-project/main:(ffc498dfcc8d...)$ git switch --detach HEAD^
Previous HEAD position was ffc498dfcc8d Align definition of DW_OP_plus with DWARF spec [3/3]
HEAD is now at 3f250380d4bd Corrected some comment typos; NFC.
user@host:~/llvm-project/main:(3f250380d4bd...)$ ninja -C release/ opt
ninja: Entering directory `release/'
...
[149/149] Linking CXX executable bin/opt
user@host:~/llvm-project/main:(3f250380d4bd...)$ release/bin/opt ../f.bc -S
opt: /home/slinder1/llvm-project/main/llvm/lib/Bitcode/Reader/MetadataLoader.cpp:363: (anonymous namespace)::(anonymous namespace):
:PlaceholderQueue::~PlaceholderQueue(): Assertion `empty() && "PlaceholderQueue hasn't been flushed before being destroyed"' failed
.
#0 0x0000000002aab1b4 PrintStackTraceSignalHandler(void*) Signals.cpp:0:0
#1 0x0000000002aab4f6 SignalHandler(int) Signals.cpp:0:0
#2 0x00007f56a9e753c0 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x153c0)
#3 0x00007f56a970718b raise /build/glibc-eX1tMB/glibc-2.31/signal/../sysdeps/unix/sysv/linux/raise.c:51:1
#4 0x00007f56a96e6859 abort /build/glibc-eX1tMB/glibc-2.31/stdlib/abort.c:81:7
#5 0x00007f56a96e6729 get_sysdep_segment_value /build/glibc-eX1tMB/glibc-2.31/intl/loadmsgcat.c:509:8
#6 0x00007f56a96e6729 _nl_load_domain /build/glibc-eX1tMB/glibc-2.31/intl/loadmsgcat.c:970:34
#7 0x00007f56a96f7f36 (/lib/x86_64-linux-gnu/libc.so.6+0x36f36)
#8 0x0000000002f8e298 llvm::MetadataLoader::MetadataLoaderImpl::parseMetadata(bool) (release/bin/opt+0x2f8e298)
#9 0x0000000002f94dcc llvm::MetadataLoader::parseMetadata(bool) (release/bin/opt+0x2f94dcc)
#10 0x0000000002f6ecc6 (anonymous namespace)::BitcodeReader::parseFunctionBody(llvm::Function*) BitcodeReader.cpp:0:0
#11 0x0000000002f6cf29 (anonymous namespace)::BitcodeReader::materialize(llvm::GlobalValue*) BitcodeReader.cpp:0:0
#12 0x0000000002f6dc1a (anonymous namespace)::BitcodeReader::materializeModule() BitcodeReader.cpp:0:0
#13 0x00000000025a7dda llvm::Module::materializeAll() (release/bin/opt+0x25a7dda)
#14 0x0000000002f66dd3 llvm::BitcodeModule::getModuleImpl(llvm::LLVMContext&, bool, bool, bool) (release/bin/opt+0x2f66dd3)
#15 0x0000000002f6a519 llvm::parseBitcodeFile(llvm::MemoryBufferRef, llvm::LLVMContext&) (release/bin/opt+0x2f6a519)
#16 0x00000000026c7cf1 llvm::parseIR(llvm::MemoryBufferRef, llvm::SMDiagnostic&, llvm::LLVMContext&) (release/bin/opt+0x26c7cf1)
#17 0x00000000026c81e0 llvm::parseIRFile(llvm::StringRef, llvm::SMDiagnostic&, llvm::LLVMContext&) (release/bin/opt+0x26c81e0)
#18 0x00000000014d65fc main (release/bin/opt+0x14d65fc)
#19 0x00007f56a96e80b3 __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:342:3
#20 0x00000000014cc5ae _start (release/bin/opt+0x14cc5ae)
Stack dump:
0.      Program arguments: release/bin/opt ../f.bc -S
Aborted (core dumped)
user@host:~/llvm-project/main:(3f250380d4bd...)$

Again, my guess is that we just want to not assert in this case, and "clear" the PlaceholderQueue as none of the Metadata we have parsed will be used.

The issue of testing newly-encoded bitcode against previous versions of the loader seems like a trickier issue, though. In fact, it may be tricky to test the fix for this assert at all, as I assume it requires manipulating bitcode files to make them invalid in some way that would trigger these return("Invalid record")s.

Any thoughts on where to go from here, and whether this patch should be blocked on these fixes?

[dexonsmith]

In D104827#2855970, @dexonsmith wrote:

That looks bad — best to revert while that's investigated.

Oops... I had misread this comment:

In D104827#2854825, @uabelho wrote:

We've noticed that this patch changes things so bcfiles created with the patch cannot be parsed with binaries built before it.

I don't think we have any guarantees about reading new bitcode with old readers. There's some machinery for a high-level version check to get a nice error (checking MODULE_CODE_VERSION) but I think that's it. (I thought this was saying the new reader couldn't read the old bitcode, in which case there'd be a missing bitcode upgrade.)

In D104827#2864582, @scott.linder wrote:

Any thoughts on where to go from here, and whether this patch should be blocked on these fixes?

Assuming IIUC now, the patch can probably land as-is... but probably makes sense to investigate the LLDB issue first. @JDevlieghere, could the failing LLDB test somehow be reading a new bitcode file with an old reader, or is that something different?

[uabelho]

In D104827#2865292, @dexonsmith wrote:

In D104827#2855970, @dexonsmith wrote:

That looks bad — best to revert while that's investigated.

Oops... I had misread this comment:

In D104827#2854825, @uabelho wrote:

We've noticed that this patch changes things so bcfiles created with the patch cannot be parsed with binaries built before it.

I don't think we have any guarantees about reading new bitcode with old readers. There's some machinery for a high-level version check to get a nice error (checking MODULE_CODE_VERSION) but I think that's it. (I thought this was saying the new reader couldn't read the old bitcode, in which case there'd be a missing bitcode upgrade.)

Hm, I often do this and I can't remember it failing before. E.g. if I run into a crash in opt, I usually save the bc file and bisect into old versions to see with what opt version the crash starts appearing. This usually works very well so I was surprised and considered it a bug now when it didn't work.

In D104827#2864582, @scott.linder wrote:

Any thoughts on where to go from here, and whether this patch should be blocked on these fixes?

Assuming IIUC now, the patch can probably land as-is... but probably makes sense to investigate the LLDB issue first. @JDevlieghere, could the failing LLDB test somehow be reading a new bitcode file with an old reader, or is that something different?

I've seen the same crash in our testing and then there was no "new bitcode with old reader" stuff involved, it was just a normal run crashing in somewhere llc (for our out of tree target).

[scott.linder]

llvm/lib/IR/AsmWriter.cpp

4737–4745

I think I've identified the source of the bug causing the assert on GreenDragon: I didn't intend to remove this FromValue=true argument. I believe the assert can still be hoisted up, so I'll post an updated patch which just restores this line.

[scott.linder]

Fix bug in printMetadataImpl

[scott.linder]

Eliminate the getDistinct method entirely from DIExpression and DIArgList. I
went about this by splitting up the DEFINE_MDNODE_* macros so each MDNode
class can use an appropriate version based on their distinct/uniqued
properties.

Also fix up the end of DebugInfoMetadata.h to #undef all of the macros.

[scott.linder]

Rebase

I believe the only pending issue is resolved, so I plan to land this tomorrow
if nobody has any objections.

[aeubanks]

hi, this is causing crashes

$ build/rel/bin/opt -passes='thinlto-pre-link<O2>' /tmp/a.ll -o /dev/null
opt: ../../llvm/include/llvm/IR/Metadata.def:180: void (anonymous namespace)::ModuleBitcodeWriter::writeMetadataRecords(ArrayRef<const llvm::Metadata *>, SmallVectorImpl<uint64_t> &, std::vector<
unsigned int> *, std::vector<uint64_t> *): Assertion `!N->isDistinct() && "Expected non-distinct " "DIArgList"' failed.                                                                            
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.                                                                                                              
Stack dump:                                                                                                                                                                                        
0.      Program arguments: build/rel/bin/opt -passes=thinlto-pre-link<O2> /tmp/a.ll -o /dev/null                                                                                                   
 #0 0x0000000001f37a13 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /usr/local/google/home/aeubanks/repos/llvm-project/build/rel/../../llvm/lib/Support/Unix/Signals.inc:565:13             
 #1 0x0000000001f3588e llvm::sys::RunSignalHandlers() /usr/local/google/home/aeubanks/repos/llvm-project/build/rel/../../llvm/lib/Support/Signals.cpp:98:18                                        
 #2 0x0000000001f37d7f SignalHandler(int) /usr/local/google/home/aeubanks/repos/llvm-project/build/rel/../../llvm/lib/Support/Unix/Signals.inc:407:1                                               
 #3 0x00007f2003b8e8e0 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x138e0)                                                                                                                
 #4 0x00007f2003668e71 raise ./signal/../sysdeps/unix/sysv/linux/raise.c:50:1                                                                                                                      
 #5 0x00007f2003652536 abort ./stdlib/abort.c:81:7                                                                                                                                                 
 #6 0x00007f200365241f get_sysdep_segment_value ./intl/loadmsgcat.c:509:8                                                                                                                          
 #7 0x00007f200365241f _nl_load_domain ./intl/loadmsgcat.c:970:34                                                                                                                                  
 #8 0x00007f20036617f2 (/lib/x86_64-linux-gnu/libc.so.6+0x357f2)                                                                                                                                   
 #9 0x000000000168e506 (anonymous namespace)::ModuleBitcodeWriter::writeMetadataRecords(llvm::ArrayRef<llvm::Metadata const*>, llvm::SmallVectorImpl<unsigned long>&, std::vector<unsigned int, std::allocator<unsigned int> >*, std::vector<unsigned long, std::allocator<unsigned long> >*) /usr/local/google/home/aeubanks/repos/llvm-project/build/rel/../../llvm/include/llvm/IR/Metadata.def:0:0
#10 0x000000000167df15 writeFunctionMetadata /usr/local/google/home/aeubanks/repos/llvm-project/build/rel/../../llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:2303:3
#11 0x000000000167df15 writeFunction /usr/local/google/home/aeubanks/repos/llvm-project/build/rel/../../llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:3351:3
#12 0x000000000167df15 (anonymous namespace)::ModuleBitcodeWriter::write() /usr/local/google/home/aeubanks/repos/llvm-project/build/rel/../../llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:4448:7
#13 0x0000000001677d32 ~ModuleBitcodeWriterBase /usr/local/google/home/aeubanks/repos/llvm-project/build/rel/../../llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:155:7
#14 0x0000000001677d32 llvm::BitcodeWriter::writeModule(llvm::Module const&, bool, llvm::ModuleSummaryIndex const*, bool, std::array<unsigned int, 5ul>*) /usr/local/google/home/aeubanks/repos/llvm-project/build/rel/../../llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:4626:1
#15 0x00000000016813cb llvm::WriteBitcodeToFile(llvm::Module const&, llvm::raw_ostream&, bool, llvm::ModuleSummaryIndex const*, bool, std::array<unsigned int, 5ul>*) /usr/local/google/home/aeubanks/repos/llvm-project/build/rel/../../llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:4653:10

a.ll60 KBDownload

(I tried reducing as much as possible but there's still a lot of debug info)

[scott.linder]

In D104827#3119839, @aeubanks wrote:

hi, this is causing crashes

$ build/rel/bin/opt -passes='thinlto-pre-link<O2>' /tmp/a.ll -o /dev/null
opt: ../../llvm/include/llvm/IR/Metadata.def:180: void (anonymous namespace)::ModuleBitcodeWriter::writeMetadataRecords(ArrayRef<const llvm::Metadata *>, SmallVectorImpl<uint64_t> &, std::vector<
unsigned int> *, std::vector<uint64_t> *): Assertion `!N->isDistinct() && "Expected non-distinct " "DIArgList"' failed.                                                                            
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.                                                                                                              
Stack dump:                                                                                                                                                                                        
0.      Program arguments: build/rel/bin/opt -passes=thinlto-pre-link<O2> /tmp/a.ll -o /dev/null                                                                                                   
 #0 0x0000000001f37a13 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /usr/local/google/home/aeubanks/repos/llvm-project/build/rel/../../llvm/lib/Support/Unix/Signals.inc:565:13             
 #1 0x0000000001f3588e llvm::sys::RunSignalHandlers() /usr/local/google/home/aeubanks/repos/llvm-project/build/rel/../../llvm/lib/Support/Signals.cpp:98:18                                        
 #2 0x0000000001f37d7f SignalHandler(int) /usr/local/google/home/aeubanks/repos/llvm-project/build/rel/../../llvm/lib/Support/Unix/Signals.inc:407:1                                               
 #3 0x00007f2003b8e8e0 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x138e0)                                                                                                                
 #4 0x00007f2003668e71 raise ./signal/../sysdeps/unix/sysv/linux/raise.c:50:1                                                                                                                      
 #5 0x00007f2003652536 abort ./stdlib/abort.c:81:7                                                                                                                                                 
 #6 0x00007f200365241f get_sysdep_segment_value ./intl/loadmsgcat.c:509:8                                                                                                                          
 #7 0x00007f200365241f _nl_load_domain ./intl/loadmsgcat.c:970:34                                                                                                                                  
 #8 0x00007f20036617f2 (/lib/x86_64-linux-gnu/libc.so.6+0x357f2)                                                                                                                                   
 #9 0x000000000168e506 (anonymous namespace)::ModuleBitcodeWriter::writeMetadataRecords(llvm::ArrayRef<llvm::Metadata const*>, llvm::SmallVectorImpl<unsigned long>&, std::vector<unsigned int, std::allocator<unsigned int> >*, std::vector<unsigned long, std::allocator<unsigned long> >*) /usr/local/google/home/aeubanks/repos/llvm-project/build/rel/../../llvm/include/llvm/IR/Metadata.def:0:0
#10 0x000000000167df15 writeFunctionMetadata /usr/local/google/home/aeubanks/repos/llvm-project/build/rel/../../llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:2303:3
#11 0x000000000167df15 writeFunction /usr/local/google/home/aeubanks/repos/llvm-project/build/rel/../../llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:3351:3
#12 0x000000000167df15 (anonymous namespace)::ModuleBitcodeWriter::write() /usr/local/google/home/aeubanks/repos/llvm-project/build/rel/../../llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:4448:7
#13 0x0000000001677d32 ~ModuleBitcodeWriterBase /usr/local/google/home/aeubanks/repos/llvm-project/build/rel/../../llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:155:7
#14 0x0000000001677d32 llvm::BitcodeWriter::writeModule(llvm::Module const&, bool, llvm::ModuleSummaryIndex const*, bool, std::array<unsigned int, 5ul>*) /usr/local/google/home/aeubanks/repos/llvm-project/build/rel/../../llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:4626:1
#15 0x00000000016813cb llvm::WriteBitcodeToFile(llvm::Module const&, llvm::raw_ostream&, bool, llvm::ModuleSummaryIndex const*, bool, std::array<unsigned int, 5ul>*) /usr/local/google/home/aeubanks/repos/llvm-project/build/rel/../../llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:4653:10

a.ll60 KBDownload

(I tried reducing as much as possible but there's still a lot of debug info)

Thank you for the reproducer! It seems like one of my fundamental assumptions about how LLVM handles metadata was faulty.

I assumed that uniqued nodes couldn't "become distinct", but they evidently can due to things like RAUW. I'll try to get a better grasp on what the rules actually are and then update the review.

[scott.linder]

Update DIArgList::handleChangedOperand to propagate in-place changes which
would require the DIArgList become distinct up to the owning MetadataAsValue.

This feels a bit odd, but so does allowing DIArgList to be distinct as an
implementation detail, while never serializing that fact (and so making a
round-trip lossy). Semantically this may be OK, but unless we have a good
reason not to I feel like finding a way to ensure these "inline" nodes are
always uniqued will make the relationship between IR/bitcode and the in-memory
representation more obvious.

[scott.linder]

In D104827#3128572, @scott.linder wrote:

Update DIArgList::handleChangedOperand to propagate in-place changes which
would require the DIArgList become distinct up to the owning MetadataAsValue.

This feels a bit odd, but so does allowing DIArgList to be distinct as an
implementation detail, while never serializing that fact (and so making a
round-trip lossy). Semantically this may be OK, but unless we have a good
reason not to I feel like finding a way to ensure these "inline" nodes are
always uniqued will make the relationship between IR/bitcode and the in-memory
representation more obvious.

I'm adding some more reviewers from those involved in https://reviews.llvm.org/D108968, as I'm essentially extending the changes made there.

[dexonsmith]

In D104827#3128572, @scott.linder wrote:

Update DIArgList::handleChangedOperand to propagate in-place changes which
would require the DIArgList become distinct up to the owning MetadataAsValue.

This feels a bit odd, but so does allowing DIArgList to be distinct as an
implementation detail, while never serializing that fact (and so making a
round-trip lossy).

Seems reasonable to try to fix this.

Semantically this may be OK, but unless we have a good
reason not to I feel like finding a way to ensure these "inline" nodes are
always uniqued will make the relationship between IR/bitcode and the in-memory
representation more obvious.

The tricky constraint is that (most) references to Metadata are not in any use-list.

llvm/lib/IR/DebugInfoMetadata.cpp

1679–1686

I'm sorry; I'm lacking some context here; I'd appreciate a bit of hand-holding.

I see how this updates the MetadataAsValue reference.

Can you walk me through in way way it's enforced that DIArgList is only referenced by MetadataAsValue? It sounds like textual IR has something, but can you confirm? What about the C++ API? The verifier? And what about historical bitcode... i.e., what can be found there?

• If there are holes in enforcement somehow, what's going to happen to the dangling references to this node?
• Can/should we delete this node now? And/or somehow mark it as dead, so that ValueAsMetadata will look through it?

[scott.linder]

llvm/lib/IR/DebugInfoMetadata.cpp

1679–1686

Can you walk me through in way way it's enforced that DIArgList is only referenced by MetadataAsValue?

I don't know that it is enforced currently, but I believe it was an intended restriction. The LLVM IR langref entry for DIArgList currently says (emphasis mine):

Because a DIArgList may refer to local values within a function, it must only be used as a function argument, must always be inlined, and cannot appear in named metadata.

The only way I can interpret the bold portion is to make it equivalent to "it must only be used through a MetadataAsValue", as that is the only means of using metadata as a function argument.

It sounds like textual IR has something, but can you confirm? What about the C++ API? The verifier? And what about historical bitcode... i.e., what can be found there?

Would it be appropriate to just add as a verifier check? No matter how the IR is produced it would then fail to verify, so we would cover any current or future APIs. For bitcode I've implemented an upgrade path to force IsDistinct=false when loading old bitcode.

If there are holes in enforcement somehow, what's going to happen to the dangling references to this node?
Can/should we delete this node now? And/or somehow mark it as dead, so that ValueAsMetadata will look through it?

I need to look closer at the lifetime of metadata nodes, it seems clear there is more to do here I'm just not sure exactly what that is.

[StephenTozer]

llvm/lib/IR/DebugInfoMetadata.cpp

1679–1686

Can you walk me through in way way it's enforced that DIArgList is only referenced by MetadataAsValue? It sounds like textual IR has something, but can you confirm? What about the C++ API? The verifier? And what about historical bitcode... i.e., what can be found there?

In textual IR parsing, a DIArgList cannot be parsed without a reference to PerFunctionState, which is provided for metadata exclusively when parsing the contents of MetadataAsValue. There is no explicit assertion to this effect, and there is also no assertion in Bitcode parsing or in the verifier either - this should probably be added! Although I don't think there should be any existing cases of this, as the Bitcode writer can also only write DIArgLists as part of a MetadataAsValue and this has been true since the metadata was added, so the only possible scenario would be someone manually editing a bitcode file I think.

[StephenTozer]

llvm/lib/IR/DebugInfoMetadata.cpp

1679–1686

Just to clarify, because it's slightly unclear in my last comment: I believe that the right move is to just add a verifier check. It doesn't really make sense for a DIArgList to exist outside of a MetadataAsValue in any circumstance, as it may refer to local function values and so must exist as a Value within a function, and the only way for that to happen is by being wrapped in a MetadataAsValue. As such, I don't think we need to put assertions all over the place to catch this, only to express it in the verifier as a rule of IR that will likely never be hit.

The equivalent assertion for LocalAsMetadata, which must be wrapped by MetadataAsValue for the same reason, is in Verifier::visitMDNode. I've also noticed while checking this that Verifier::visitMetadataAsValue does not call Verifier::visitDIArgList if it contains one, which should also be adjusted (this doesn't need to be a part of this patch).

[dexonsmith]

llvm/lib/IR/DebugInfoMetadata.cpp

1679–1686

Relying on the verifier is probably fine. I suspect you could add verifier coverage in a unit test (i.e., DIArgList in an invalid spot and then run the verifier), maybe in llvm/unittests/IR/MetadataTest.cpp or DebugInfoTest.cpp. It looks like that's missing for LocalAsMetadata (my fault I'm sure).

Still, IMO an assertion in MDOperand that the argument is valid to reference (i.e., neither LocalAsMetadata nor DIArgList) would be nice to have. Maybe Metadata could grow:

bool isValidOperand() const { return Kind != ... };

and then MDOperand::reset could check:

assert((!MD || MD->isValidMDOperand()) && "...");

If there are holes in enforcement somehow, what's going to happen to the dangling references to this node?
Can/should we delete this node now? And/or somehow mark it as dead, so that ValueAsMetadata will look through it?

I need to look closer at the lifetime of metadata nodes, it seems clear there is more to do here I'm just not sure exactly what that is.

Looking at the example of LocalAsMetadata, since DIArgList seems similar, ValueAsMetadata::handleRAUW() deletes the node after RAUW'ing it. It looks like DIArgList doesn't inherit from ReplaceableMetadataImpl so it can't RAUW itself, but maybe it should?

[scott.linder]

Add enforcement of the always-owned-by-MetadataAsValue property of DIArgList to
the metadata verifier. I'm not sure this is sufficient, but at least it is a
start.

Fixed the handling of the uniquify()!=this case in
DIArgList::handleChangedOperand to assert if the above property doesn't hold.

Add unittests for both changes above; the reproducers for this in IR rely on
specific sequences of optimizations, and would be much more fragile.

Is this moving in the right direction? Does the restriction on DIArgList seem
reasonable, and if so does it need more explicit documentation or more thorough
enforcement?

[scott.linder]

In D104827#3136642, @scott.linder wrote:

Add enforcement of the always-owned-by-MetadataAsValue property of DIArgList to
the metadata verifier. I'm not sure this is sufficient, but at least it is a
start.

Fixed the handling of the uniquify()!=this case in
DIArgList::handleChangedOperand to assert if the above property doesn't hold.

Add unittests for both changes above; the reproducers for this in IR rely on
specific sequences of optimizations, and would be much more fragile.

Is this moving in the right direction? Does the restriction on DIArgList seem
reasonable, and if so does it need more explicit documentation or more thorough
enforcement?

I pushed this before looking at the intervening comments, so the last bit is a bit redundant; it sounds like the restriction on DIArgList was intended and makes sense, so I think the remaining question is whether the Verifier check is sufficient.

[dexonsmith]

In D104827#3137807, @scott.linder wrote:

Is this moving in the right direction? Does the restriction on DIArgList seem
reasonable, and if so does it need more explicit documentation or more thorough
enforcement?

I pushed this before looking at the intervening comments, so the last bit is a bit redundant; it sounds like the restriction on DIArgList was intended and makes sense, so I think the remaining question is whether the Verifier check is sufficient.

It seems like DIArgList is very similar to LocalAsMetadata. I feel like it'd be less error-prone for it to use/inherit-from ReplaceableMetadataImpl, just like ValueAsMetadata does. Then it has the machinery to RAUW/delete itself when necessary.

(Moreover, given that it doesn't have any operands, I find it suspicious that it inherits (albeit indirectly) from MDNode. It should probably be a direct subclass of Metadata. This would mean it couldn't inherit from DINode which might complicate some debug info code; if you're going to clean that up I suggest doing it later as a separate patch.)

[scott.linder]

In D104827#3138627, @dexonsmith wrote:

In D104827#3137807, @scott.linder wrote:

Is this moving in the right direction? Does the restriction on DIArgList seem
reasonable, and if so does it need more explicit documentation or more thorough
enforcement?

I pushed this before looking at the intervening comments, so the last bit is a bit redundant; it sounds like the restriction on DIArgList was intended and makes sense, so I think the remaining question is whether the Verifier check is sufficient.

It seems like DIArgList is very similar to LocalAsMetadata. I feel like it'd be less error-prone for it to use/inherit-from ReplaceableMetadataImpl, just like ValueAsMetadata does. Then it has the machinery to RAUW/delete itself when necessary.

(Moreover, given that it doesn't have any operands, I find it suspicious that it inherits (albeit indirectly) from MDNode. It should probably be a direct subclass of Metadata. This would mean it couldn't inherit from DINode which might complicate some debug info code; if you're going to clean that up I suggest doing it later as a separate patch.)

I looked into deriving DIArgList from ReplaceableMetadataImpl, and it does get awkward quickly that it is an MDNode. It seems like there is a fair amount of code around tracking/RAUW which relies on MDNodes being eventually "resolveable". The result is that I don't think I can inherit from ReplaceableMetadataImpl without also changing the inheritence from MDNode to Metadata as you suggest.

I'll look at what effect that has on any debug-info related code, but I do worry that no matter what there will be a fair amount of special-casing.

[scott.linder]

I'm pushing the (rough) patch which tries to make the switch from inheriting
DIArglist from MDNode to ReplaceableMetadataImpl.

The biggest issue I see with a change along these lines is that the
!DIArgList syntax becomes misleading. If we really want this, I think it
implies a bit more work, namely generalizing this to something closer to a
ValuesAsMetadata (plural), with a syntax closer to:

metadata {i32 1, i64 %x, float %f}

I think this doesn't add any ambiguity to the syntax, but this is just my
initial guess at what might make sense.

I'm not sure I can commit to this work, but I would still like to propose making
the UNIQUED/DISTINCT change for DIExpression and DICompileUnit. Would it be
reasonable for me to just remove DIArgList from my original patch, as it has
proven to be special enough not to generalize how I had hoped?

[scott.linder]

I went ahead with just leaving DIArgList alone, let me know what you think

[dexonsmith]

In D104827#3189622, @scott.linder wrote:

I'm pushing the (rough) patch which tries to make the switch from inheriting
DIArglist from MDNode to ReplaceableMetadataImpl.

The biggest issue I see with a change along these lines is that the
!DIArgList syntax becomes misleading.

I don't see what's misleading about it. ! is the indicator that "metadata follows", and DIArgList is the name of this special kind of metadata.

TBH, I find it odd that there'd be something inheriting from MDNode that is as special as this....

If we really want this, I think it
implies a bit more work, namely generalizing this to something closer to a
ValuesAsMetadata (plural), with a syntax closer to:

metadata {i32 1, i64 %x, float %f}

I think this doesn't add any ambiguity to the syntax, but this is just my
initial guess at what might make sense.

This looks awkwardly similar to generic MDNode syntax (just with the distinction that generic metadata is not allowed to reference local variables). I don't think generic syntax is better than naming the type here. If you still disagree, can you explain why?

I'm not sure I can commit to this work, but I would still like to propose making
the UNIQUED/DISTINCT change for DIExpression and DICompileUnit. Would it be
reasonable for me to just remove DIArgList from my original patch, as it has
proven to be special enough not to generalize how I had hoped?

IIUC, DIExpression is data-driven, so it's reasonable for it always to be uniqued. Originally it was not going to inherit from MDNode, but IIRC it was useful as an incremental step for the transition from all generic metadata. At some point there was a FIXME in the code to reparent directly under Metadata but probably that was dropped eventually. I still think that'd be a reasonable thing to do.

If DICompileUnit already has this property, seems to me like that part could be separated into an NFC change to make it easier to review what's actually changing.

Seems reasonable to handle DIArgList separately from the other two. It'd be a shame to drop it entirely; it seems like there are some fundamental modelling problems with DIArgList that would be valuable to iron out.

llvm/lib/Bitcode/Reader/MetadataLoader.cpp

559

I don't think you need to change upgradeDIExpression.

1144

Seems odd that this is declared here. Would be nice at some point to make it local to each case below that needs it in an NFC change.

2003

I think you should replace this line with a comment, maybe something like:

// DIExpression is never distinct. Old bitcode uses the 0-bit for that
// but it's now ignored.

You can drop the assignment to IsDistinct entirely; it's not interesting here anymore.

2008–2010

I don't think you need this change.

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

2017

I'm not sure we need a new version.

llvm/lib/CodeGen/MIRParser/MIParser.cpp

1187

I'm not sure what you mean by this comment. What is "this", and why should a metadata kind being uniqued-by-content influence it?

2331

I'm not sure what you mean by this comment. What is "this", and why should a metadata kind being uniqued-by-content influence it?

llvm/lib/IR/AsmWriter.cpp

1232–1236

I don't think nodes being uniqued-by-content should force them to be inlined everywhere. Those seem like independent properties to me.

llvm/lib/IR/DebugInfoMetadata.cpp

1076

Can this be Storage == Uniqued?

llvm/lib/IR/Verifier.cpp

936

Nit: if we add a newline here, should be in a separate NFC commit since there's no other change around.

[scott.linder]

In D104827#3140973, @scott.linder wrote:

In D104827#3138627, @dexonsmith wrote:

In D104827#3137807, @scott.linder wrote:

Is this moving in the right direction? Does the restriction on DIArgList seem
reasonable, and if so does it need more explicit documentation or more thorough
enforcement?

I pushed this before looking at the intervening comments, so the last bit is a bit redundant; it sounds like the restriction on DIArgList was intended and makes sense, so I think the remaining question is whether the Verifier check is sufficient.

It seems like DIArgList is very similar to LocalAsMetadata. I feel like it'd be less error-prone for it to use/inherit-from ReplaceableMetadataImpl, just like ValueAsMetadata does. Then it has the machinery to RAUW/delete itself when necessary.

(Moreover, given that it doesn't have any operands, I find it suspicious that it inherits (albeit indirectly) from MDNode. It should probably be a direct subclass of Metadata. This would mean it couldn't inherit from DINode which might complicate some debug info code; if you're going to clean that up I suggest doing it later as a separate patch.)

I looked into deriving DIArgList from ReplaceableMetadataImpl, and it does get awkward quickly that it is an MDNode. It seems like there is a fair amount of code around tracking/RAUW which relies on MDNodes being eventually "resolveable". The result is that I don't think I can inherit from ReplaceableMetadataImpl without also changing the inheritence from MDNode to Metadata as you suggest.

I'll look at what effect that has on any debug-info related code, but I do worry that no matter what there will be a fair amount of special-casing.

In D104827#3192965, @dexonsmith wrote:

In D104827#3189622, @scott.linder wrote:

I'm pushing the (rough) patch which tries to make the switch from inheriting
DIArglist from MDNode to ReplaceableMetadataImpl.

The biggest issue I see with a change along these lines is that the
!DIArgList syntax becomes misleading.

I don't see what's misleading about it. ! is the indicator that "metadata follows", and DIArgList is the name of this special kind of metadata.

TBH, I find it odd that there'd be something inheriting from MDNode that is as special as this....

It seemed like a useful invariant that the syntax !<id> always implied an MDNode, in the same way that !"..." means MDString, although I guess this is just an implementation detail.

It may just be a naming thing, though; we say in the langref that there are only metadata "strings" and metadata "nodes", but we already have metadata "ValueAsMetadata"s too. If we were to hoist up DIArgList as well how would we want to describe it? By saying we have: strings, nodes, ValueAsMetadatas, and DIArgLists?

If we really want this, I think it
implies a bit more work, namely generalizing this to something closer to a
ValuesAsMetadata (plural), with a syntax closer to:

metadata {i32 1, i64 %x, float %f}

I think this doesn't add any ambiguity to the syntax, but this is just my
initial guess at what might make sense.

This looks awkwardly similar to generic MDNode syntax (just with the distinction that generic metadata is not allowed to reference local variables). I don't think generic syntax is better than naming the type here. If you still disagree, can you explain why?

That was my intention, because I don't see why this needs to be DI-centric. My guess was wrong either way, though, because metadata { already signifies the start of a structure type IIUC.

I was just trying to suggest that all DIArgList currently does is stand in for a function-local metadata tuple, so just making one of those and using that seems more straightforward.

I'm not sure I can commit to this work, but I would still like to propose making
the UNIQUED/DISTINCT change for DIExpression and DICompileUnit. Would it be
reasonable for me to just remove DIArgList from my original patch, as it has
proven to be special enough not to generalize how I had hoped?

IIUC, DIExpression is data-driven, so it's reasonable for it always to be uniqued. Originally it was not going to inherit from MDNode, but IIRC it was useful as an incremental step for the transition from all generic metadata. At some point there was a FIXME in the code to reparent directly under Metadata but probably that was dropped eventually. I still think that'd be a reasonable thing to do.

That FIXME is still present, and if that is true then maybe my whole conception of the class hierarchy and what to call various parts is just not lining up with reality. As a smoke test for whether I'm on the same page, would you expect DIExpression to no longer be referred to as a "node" in documentation, as it would no longer derive from MDNode?

If DICompileUnit already has this property, seems to me like that part could be separated into an NFC change to make it easier to review what's actually changing.

Sure; just to confirm, you mean split this into two patches, one NFC patch adding HANDLE_MDNODE_LEAF_DISTINCT, and another adding HANDLE_MDNODE_LEAF_UNIQUED?

Seems reasonable to handle DIArgList separately from the other two. It'd be a shame to drop it entirely; it seems like there are some fundamental modelling problems with DIArgList that would be valuable to iron out.

Yes, I agree that at the very least the fact that DIArgList can transiently become distinct, but then not round-trip this property through IR, probably deserves attention. I am not particularly confident that the way I first though about remedying it really lines up with how metadata is currently designed, but I would be interested in learning enough to contribute a fix.

I'll wait to hear if I follow how you would prefer the patch be split up before I incorporate your other feedback.

llvm/lib/Bitcode/Reader/MetadataLoader.cpp

2003

OK, fair enough; so the bit just becomes "don't care" going forward, which naturally means old bitcode is forward compatible

llvm/lib/CodeGen/MIRParser/MIParser.cpp

1187

"this" is trying to get at "this code which supports DIExpression appearing inline in MIR"; I can try to fix up the wording some to be more clear

The property I was aiming for with the whole patch was that "inlined" metadata is never distinct. This seemed like a logical "rule" that would explain the lack of syntax for distinct nodes inline.

2331

ditto

llvm/lib/IR/DebugInfoMetadata.cpp

1076

Yes, I'll make this change.

llvm/lib/IR/Verifier.cpp

936

Woops, I just didn't notice this before pushing the diff. I'll just remove this change from the patch.

[dexonsmith]

In D104827#3193518, @scott.linder wrote:

I was just trying to suggest that all DIArgList currently does is stand in for a function-local metadata tuple, so just making one of those and using that seems more straightforward.

Ah, that's an interesting point; thanks for clarifying!

Stepping back a bit...

Before the big metadata / debug info refactor, there was support for metadata nodes owned by a function. These nodes were allowed to point at function-local values. The only uses of these metadata nodes were for direct reference from debug info intrinsics. I removed them entirely to simplify the model, and left behind LocalAsMetadata.

I've since realized that much of this "global" metadata is really tied to a specific function. Three examples:

• Inlined DILocations were made distinct in order to defeat uniquing across different function definitions.
• DISubprogram have become function attachments; and it's illegal to have the same one attached in multiple places. It's effectively owned by the function; and it needs to be cloned when a function is cloned.
• Outside of debug info, there are optimization-related metadata that describe function-local constructs, and that defeat uniquing by having a "root" node that self-references and/or is marked distinct. (IIRC, loop metadata is an example?)

I think we should bring back local metadata; moreover, there really seem to be three scopes for metadata:

• Context: pure / immutable constants; cannot change / cannot be distinct. Only reference other "context" constants (any referenced LLVM values are also pure constants (i32 0 okay, but i32* @global is not)). Should be shared across all users of an LLVMContext and owned by the LLVMContext. Since they are immutable, never "cloned" by cloneFunctionInto / cloneModule / related APIs.
• Module: module-specific. May have identity and be mutable (distinct). If it is uniqued / "constant", then it is at "module" scope because it (transitively) references something else with "module" scope (e.g., a global value or distinct metadata). Can reference LLVM global values but not local values. These are effectively owned by a specific Module. Cloning a module would require making new copies of these, but cloning a function would not.
• Local: function-specific. May have identity and be mutable (distinct). If it is uniqued / "constant", then it is at "local" scope because it (transitively) references something else with "local" scope. Cloning a function would require making new copies of these. Effectively owned by the function/global value that it's associated with.

In this model, it seems like DIArgList falls under "local"; and you're suggesting it's just a stand-in for not having a local "MDNode", and not really semantically tied to debug info or argument lists. If that's the case, then it seems they're only required to be inline right now because there's no syntax to describe them otherwise. Syntax could look like:

define void @f(i32 %a, i32 %b) {
  call @llvm.some.intrinsic(!%1)
  ; Some local metadata. Can reference LocalAsMetadata.
  ; Cloned when the function is cloned.
  !%1 = metadata !{i32 %a, i32 %b, !%2}
  !%2 = distinct metadata !{!1}
}

; Global metadata. Cloned when a module is cloned.
!1 = distinct metadata !{!2, ptr @f}
!2 = metadata !{!1, !3}

; Context metadata. Pure constants.
!3 = metadata !{!4, !5}
!4 = metadata !{i32 1, ptr null}
!5 = metadata !{}

Stepping back again, I think there are three kinds of identity for metadata:

• Distinct: not in any uniquing table (e.g., MDNode marked distinct)
• Uniqued (always): in a uniquing table; either immutable (e.g., "Context" scope / DIExpression) or has permanent RAUW support to handle changes (e.g., ValueAsMetadata)
• Uniqued (best-effort): findable via uniquing tables, but operands might change and lacks permanent RAUW support; degrades to "distinct" if an operand changes (e.g., MDNode without distinct)

The current design for metadata optimizes for having most metadata either "distinct" or "best-effort-uniqued". Permanent RAUW support is expensive (in this design).

Our recent discussions about DIArgList point toward making it always-uniqued; since the operands can change, this requires permanent RAUW support. I wonder if, instead, best-effort-uniqued would be sufficient... if only there were syntax for it. WDYT?

Sure; just to confirm, you mean split this into two patches, one NFC patch adding HANDLE_MDNODE_LEAF_DISTINCT, and another adding HANDLE_MDNODE_LEAF_UNIQUED?

Yes.

For the former, maybe "ALWAYS_DISTINCT" would be more clear? Either way, this seems like a well-defined concept.

For the latter, I'm not quite convinced yet. The name doesn't seem right, since most nodes are stored/de-duped in uniquing tables (maybe "ALWAYS_UNIQUED" would work?). But:

• I don't think "always-uniqued" should imply things like "printed inline in textual IR"
• I'm not sure should have MDNodes that have permanent RAUW support (since it's expensive)
  • Can we get away with a "local" MDNode that is best-effort-uniqued?

llvm/lib/CodeGen/MIRParser/MIParser.cpp

1187

We could support all nodes here for parsing, if we think that's useful... I'm not sure if there's a good reason to only have DIExpression here...

that would explain the lack of syntax for distinct nodes inline.

IIRC, the historical lack of syntax (in .ll at least, not sure about .mir) is because it didn't seem useful/important. When MDNodes are distinct, it is because they either need identity *a priori* (in which case, probably there are multiple references to them and they MUST be out-of-line) or there was a uniquing table collision for best-effort-uniqued metadata (expected to be rare). Anything that was always-uniqued had first class syntax.

llvm/docs/LangRef.rst

 These aren't inherently debug info centric, but currently all the specialized
 metadata nodes are related to debug info.
 
 .. _DICompileUnit:
 
 DICompileUnit
 """""""""""""
 
-``DICompileUnit`` nodes represent a compile unit. The ``enums:``,
-``retainedTypes:``, ``globals:``, ``imports:`` and ``macros:`` fields are tuples
-containing the debug info to be emitted along with the compile unit, regardless
-of code optimizations (some nodes are only emitted if there are references to
-them from instructions). The ``debugInfoForProfiling:`` field is a boolean
-indicating whether or not line-table discriminators are updated to provide
-more-accurate debug info for profiling results.
+``DICompileUnit`` nodes represent a compile unit. ``DICompileUnit`` nodes must
+be ``distinct``. The ``enums:``, ``retainedTypes:``, ``globals:``, ``imports:``
+and ``macros:`` fields are tuples containing the debug info to be emitted along
+with the compile unit, regardless of code optimizations (some nodes are only
+emitted if there are references to them from instructions). The
+``debugInfoForProfiling:`` field is a boolean indicating whether or not
+line-table discriminators are updated to provide more-accurate debug info for
+profiling results.
 
 .. code-block:: text
 
-    !0 = !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang",
+    !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang",
                                  isOptimized: true, flags: "-O2", runtimeVersion: 2,
                                  splitDebugFilename: "abc.debug", emissionKind: FullDebug,
                                  enums: !2, retainedTypes: !3, globals: !4, imports: !5,
                                  macros: !6, dwoId: 0x0abcd)
 
 Compile unit descriptors provide the root scope for objects declared in a
 specific compilation unit. File descriptors are defined using this scope.  These
 descriptors are collected by a named metadata node ``!llvm.dbg.cu``. They keep
 track of global variables, type information, and imported entities (declarations
 and namespaces).
 
 .. _DIFile:
.. code-block:: text
     !2 = !DILocalVariable(name: "y", scope: !5, file: !2, line: 7, type: !3)
 
 .. _DIExpression:
 
 DIExpression
 """"""""""""
 
 ``DIExpression`` nodes represent expressions that are inspired by the DWARF
-expression language. They are used in :ref:`debug intrinsics<dbg_intrinsics>`
-(such as ``llvm.dbg.declare`` and ``llvm.dbg.value``) to describe how the
-referenced LLVM variable relates to the source language variable. Debug
-intrinsics are interpreted left-to-right: start by pushing the value/address
-operand of the intrinsic onto a stack, then repeatedly push and evaluate
-opcodes from the DIExpression until the final variable description is produced.
+expression language. ``DIExpression`` nodes must not be ``distinct``, and are
+canonically printed inline at each use. They are used in :ref:`debug
+intrinsics<dbg_intrinsics>` (such as ``llvm.dbg.declare`` and
+``llvm.dbg.value``) to describe how the referenced LLVM variable relates to the
+source language variable. Debug intrinsics are interpreted left-to-right: start
+by pushing the value/address operand of the intrinsic onto a stack, then
+repeatedly push and evaluate opcodes from the DIExpression until the final
+variable description is produced.
 
 The current supported opcode vocabulary is limited:
 
 - ``DW_OP_deref`` dereferences the top of the expression stack.
 - ``DW_OP_plus`` pops the last two entries from the expression stack, adds
   them together and appends the result to the expression stack.
 - ``DW_OP_minus`` pops the last two entries from the expression stack, subtracts
   the last entry from the second last entry and appends the result to the
- ``DW_OP_LLVM_implicit_pointer`` It specifies the dereferenced value. It can
   it points to is known. This operator is required as it is different than DWARF
   operator DW_OP_implicit_pointer in representation and specification (number
   and types of operands) and later can not be used as multiple level.
 
 .. code-block:: text
 
     IR for "*ptr = 4;"
     --------------
-    call void @llvm.dbg.value(metadata i32 4, metadata !17, metadata !20)
+    call void @llvm.dbg.value(metadata i32 4, metadata !17,
+                              metadata !DIExpression(DW_OP_LLVM_implicit_pointer)))
     !17 = !DILocalVariable(name: "ptr1", scope: !12, file: !3, line: 5,
                            type: !18)
     !18 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !19, size: 64)
     !19 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
-    !20 = !DIExpression(DW_OP_LLVM_implicit_pointer))
 
     IR for "**ptr = 4;"
     --------------
-    call void @llvm.dbg.value(metadata i32 4, metadata !17, metadata !21)
+    call void @llvm.dbg.value(metadata i32 4, metadata !17,
+                              metadata !DIExpression(DW_OP_LLVM_implicit_pointer,
+                                                     DW_OP_LLVM_implicit_pointer)))
     !17 = !DILocalVariable(name: "ptr1", scope: !12, file: !3, line: 5,
                            type: !18)
     !18 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !19, size: 64)
     !19 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !20, size: 64)
     !20 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
-    !21 = !DIExpression(DW_OP_LLVM_implicit_pointer,
-                        DW_OP_LLVM_implicit_pointer))
 
 DWARF specifies three kinds of simple location descriptions: Register, memory,
 and implicit location descriptions.  Note that a location description is
 defined over certain ranges of a program, i.e the location of a variable may
 change over the course of the program. Register and memory location
 descriptions describe the *concrete location* of a source variable (in the
 sense that a debugger might modify its value), whereas *implicit locations*
 describe merely the actual *value* of a source variable which might not exist
.. code-block:: text
     !2 = !DIExpression(DW_OP_bit_piece, 3, 7)
     !3 = !DIExpression(DW_OP_deref, DW_OP_constu, 3, DW_OP_plus, DW_OP_LLVM_fragment, 3, 7)
     !4 = !DIExpression(DW_OP_constu, 2, DW_OP_swap, DW_OP_xderef)
     !5 = !DIExpression(DW_OP_constu, 42, DW_OP_stack_value)
 
 DIArgList
 """"""""""""
 
-``DIArgList`` nodes hold a list of constant or SSA value references. These are
-used in :ref:`debug intrinsics<dbg_intrinsics>` (currently only in
+``DIArgList`` nodes hold a list of constant or SSA value references.
+``DIArgList`` must not be ``distinct``, must only be used as an argument to a
+function call, and must appear inline at each use. ``DIArgList`` may refer to
+function-local values of the containing function. ``DIArgList`` nodes are used
+in :ref:`debug intrinsics<dbg_intrinsics>` (currently only in
 ``llvm.dbg.value``) in combination with a ``DIExpression`` that uses the
-``DW_OP_LLVM_arg`` operator. Because a DIArgList may refer to local values
-within a function, it must only be used as a function argument, must always be
-inlined, and cannot appear in named metadata.
+``DW_OP_LLVM_arg`` operator.
 
 .. code-block:: text
 
     llvm.dbg.value(metadata !DIArgList(i32 %a, i32 %b),
                    metadata !16,
                    metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus))
 
 DIFlags

llvm/docs/SourceLevelDebugging.rst

 .. code-block:: text
 
   ; Function Attrs: nounwind ssp uwtable
   define void @foo() #0 !dbg !4 {
   entry:
     %X = alloca i32, align 4
     %Y = alloca i32, align 4
     %Z = alloca i32, align 4
-    call void @llvm.dbg.declare(metadata i32* %X, metadata !11, metadata !13), !dbg !14
-    store i32 21, i32* %X, align 4, !dbg !14
-    call void @llvm.dbg.declare(metadata i32* %Y, metadata !15, metadata !13), !dbg !16
-    store i32 22, i32* %Y, align 4, !dbg !16
-    call void @llvm.dbg.declare(metadata i32* %Z, metadata !17, metadata !13), !dbg !19
-    store i32 23, i32* %Z, align 4, !dbg !19
-    %0 = load i32, i32* %X, align 4, !dbg !20
-    store i32 %0, i32* %Z, align 4, !dbg !21
-    %1 = load i32, i32* %Y, align 4, !dbg !22
-    store i32 %1, i32* %X, align 4, !dbg !23
-    ret void, !dbg !24
+    call void @llvm.dbg.declare(metadata i32* %X, metadata !11, metadata !DIExpression()), !dbg !13
+    store i32 21, i32* %X, align 4, !dbg !13
+    call void @llvm.dbg.declare(metadata i32* %Y, metadata !14, metadata !DIExpression()), !dbg !15
+    store i32 22, i32* %Y, align 4, !dbg !15
+    call void @llvm.dbg.declare(metadata i32* %Z, metadata !16, metadata !DIExpression()), !dbg !18
+    store i32 23, i32* %Z, align 4, !dbg !18
+    %0 = load i32, i32* %X, align 4, !dbg !19
+    store i32 %0, i32* %Z, align 4, !dbg !20
+    %1 = load i32, i32* %Y, align 4, !dbg !21
+    store i32 %1, i32* %X, align 4, !dbg !22
+    ret void, !dbg !23
   }
 
   ; Function Attrs: nounwind readnone
   declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
 
   attributes #0 = { nounwind ssp uwtable "less-precise-fpmad"="false" "frame-pointer"="all" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
   attributes #1 = { nounwind readnone }
 
.. code-block:: text
   !5 = !DISubroutineType(types: !6)
   !6 = !{null}
   !7 = !{i32 2, !"Dwarf Version", i32 2}
   !8 = !{i32 2, !"Debug Info Version", i32 3}
   !9 = !{i32 1, !"PIC Level", i32 2}
   !10 = !{!"clang version 3.7.0 (trunk 231150) (llvm/trunk 231154)"}
   !11 = !DILocalVariable(name: "X", scope: !4, file: !1, line: 2, type: !12)
   !12 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
-  !13 = !DIExpression()
-  !14 = !DILocation(line: 2, column: 9, scope: !4)
-  !15 = !DILocalVariable(name: "Y", scope: !4, file: !1, line: 3, type: !12)
-  !16 = !DILocation(line: 3, column: 9, scope: !4)
-  !17 = !DILocalVariable(name: "Z", scope: !18, file: !1, line: 5, type: !12)
-  !18 = distinct !DILexicalBlock(scope: !4, file: !1, line: 4, column: 5)
-  !19 = !DILocation(line: 5, column: 11, scope: !18)
-  !20 = !DILocation(line: 6, column: 11, scope: !18)
-  !21 = !DILocation(line: 6, column: 9, scope: !18)
-  !22 = !DILocation(line: 8, column: 9, scope: !4)
-  !23 = !DILocation(line: 8, column: 7, scope: !4)
-  !24 = !DILocation(line: 9, column: 3, scope: !4)
+  !13 = !DILocation(line: 2, column: 9, scope: !4)
+  !14 = !DILocalVariable(name: "Y", scope: !4, file: !1, line: 3, type: !12)
+  !15 = !DILocation(line: 3, column: 9, scope: !4)
+  !16 = !DILocalVariable(name: "Z", scope: !17, file: !1, line: 5, type: !12)
+  !17 = distinct !DILexicalBlock(scope: !4, file: !1, line: 4, column: 5)
+  !18 = !DILocation(line: 5, column: 11, scope: !17)
+  !19 = !DILocation(line: 6, column: 11, scope: !17)
+  !20 = !DILocation(line: 6, column: 9, scope: !17)
+  !21 = !DILocation(line: 8, column: 9, scope: !4)
+  !22 = !DILocation(line: 8, column: 7, scope: !4)
+  !23 = !DILocation(line: 9, column: 3, scope: !4)
 
 
 This example illustrates a few important details about LLVM debugging
 information.  In particular, it shows how the ``llvm.dbg.declare`` intrinsic and
 location information, which are attached to an instruction, are applied
 together to allow a debugger to analyze the relationship between statements,
 variable definitions, and the code used to implement the function.
 
 .. code-block:: llvm
 
-  call void @llvm.dbg.declare(metadata i32* %X, metadata !11, metadata !13), !dbg !14
+  call void @llvm.dbg.declare(metadata i32* %X, metadata !11, metadata !DIExpression()), !dbg !13
     ; [debug line = 2:7] [debug variable = X]
 
 The first intrinsic ``%llvm.dbg.declare`` encodes debugging information for the
-variable ``X``.  The metadata ``!dbg !14`` attached to the intrinsic provides
+variable ``X``.  The metadata ``!dbg !13`` attached to the intrinsic provides
 scope information for the variable ``X``.
 
 .. code-block:: text
 
-  !14 = !DILocation(line: 2, column: 9, scope: !4)
+  !13 = !DILocation(line: 2, column: 9, scope: !4)
   !4 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 1, type: !5,
                               isLocal: false, isDefinition: true, scopeLine: 1,
                               isOptimized: false, retainedNodes: !2)
 
-Here ``!14`` is metadata providing `location information
+Here ``!13`` is metadata providing `location information
 <LangRef.html#dilocation>`_.  In this example, scope is encoded by ``!4``, a
 `subprogram descriptor <LangRef.html#disubprogram>`_.  This way the location
 information attached to the intrinsics indicates that the variable ``X`` is
 declared at line number 2 at a function level scope in function ``foo``.
 
 Now lets take another example.
 
 .. code-block:: llvm
 
-  call void @llvm.dbg.declare(metadata i32* %Z, metadata !17, metadata !13), !dbg !19
+  call void @llvm.dbg.declare(metadata i32* %Z, metadata !16, metadata !DIExpression()), !dbg !18
     ; [debug line = 5:9] [debug variable = Z]
 
 The third intrinsic ``%llvm.dbg.declare`` encodes debugging information for
-variable ``Z``.  The metadata ``!dbg !19`` attached to the intrinsic provides
+variable ``Z``.  The metadata ``!dbg !18`` attached to the intrinsic provides
 scope information for the variable ``Z``.
 
 .. code-block:: text
 
-  !18 = distinct !DILexicalBlock(scope: !4, file: !1, line: 4, column: 5)
-  !19 = !DILocation(line: 5, column: 11, scope: !18)
+  !17 = distinct !DILexicalBlock(scope: !4, file: !1, line: 4, column: 5)
+  !18 = !DILocation(line: 5, column: 11, scope: !17)
 
-Here ``!19`` indicates that ``Z`` is declared at line number 5 and column
-number 11 inside of lexical scope ``!18``.  The lexical scope itself resides
+Here ``!18`` indicates that ``Z`` is declared at line number 5 and column
+number 11 inside of lexical scope ``!17``.  The lexical scope itself resides
 inside of subprogram ``!4`` described above.
 
 The scope information attached with each instruction provides a straightforward
 way to find instructions covered by a scope.
 
 Object lifetime in optimized code
 =================================
 
.. code-block:: text
   entry:
     br i1 %cond, label %truebr, label %falsebr
 
   bb1:
     %value = phi i32 [ %value1, %truebr ], [ %value2, %falsebr ]
     br label %exit, !dbg !26
 
   truebr:
-    call void @llvm.dbg.value(metadata i32 %input, metadata !30, metadata !DIExpression()), !dbg !24
-    call void @llvm.dbg.value(metadata i32 1, metadata !23, metadata !DIExpression()), !dbg !24
+    call void @llvm.dbg.value(metadata i32 %input, metadata !30, metadata !DIExpression()), !dbg !23
+    call void @llvm.dbg.value(metadata i32 1, metadata !22, metadata !DIExpression()), !dbg !23
     %value1 = add i32 %input, 1
     br label %bb1
 
   falsebr:
-    call void @llvm.dbg.value(metadata i32 %input, metadata !30, metadata !DIExpression()), !dbg !24
-    call void @llvm.dbg.value(metadata i32 2, metadata !23, metadata !DIExpression()), !dbg !24
+    call void @llvm.dbg.value(metadata i32 %input, metadata !30, metadata !DIExpression()), !dbg !23
+    call void @llvm.dbg.value(metadata i32 2, metadata !22, metadata !DIExpression()), !dbg !23
     %value = add i32 %input, 2
     br label %bb1
 
   exit:
     ret i32 %value, !dbg !30
   }
 
 Here the difficulties are:
 
 * The control flow is roughly the opposite of basic block order
-* The value of the ``!23`` variable merges into ``%bb1``, but there is no PHI
+* The value of the ``!22`` variable merges into ``%bb1``, but there is no PHI
   node
 
 As mentioned above, the ``llvm.dbg.value`` intrinsics essentially form an
 imperative program embedded in the IR, with each intrinsic defining a variable
 location. This *could* be converted to an SSA form by mem2reg, in the same way
 that it uses use-def chains to identify control flow merges and insert phi
 nodes for IR Values. However, because debug variable locations are defined for
 every machine instruction, in effect every IR instruction uses every variable
 location, which would lead to a large number of debugging intrinsics being
 generated.
 
 Examining the example above, variable ``!30`` is assigned ``%input`` on both
-conditional paths through the function, while ``!23`` is assigned differing
+conditional paths through the function, while ``!22`` is assigned differing
 constant values on either path. Where control flow merges in ``%bb1`` we would
-want ``!30`` to keep its location (``%input``), but ``!23`` to become undefined
+want ``!30`` to keep its location (``%input``), but ``!22`` to become undefined
 as we cannot determine at runtime what value it should have in %bb1 without
 inserting a PHI node. mem2reg does not insert the PHI node to avoid changing
 codegen when debugging is enabled, and does not insert the other dbg.values
 to avoid adding very large numbers of intrinsics.
 
 Instead, LiveDebugValues determines variable locations when control
 flow merges. A dataflow analysis is used to propagate locations between blocks:
 when control flow merges, if a variable has the same location in all
 predecessors then that location is propagated into the successor. If the
 predecessor locations disagree, the location becomes undefined.
 
 Once LiveDebugValues has run, every block should have all valid variable
 locations described by DBG_VALUE instructions within the block. Very little
 effort is then required by supporting classes (such as
 DbgEntityHistoryCalculator) to build a map of each instruction to every
 valid variable location, without the need to consider control flow. From
 the example above, it is otherwise difficult to determine that the location
 of variable ``!30`` should flow "up" into block ``%bb1``, but that the location
-of variable ``!23`` should not flow "down" into the ``%exit`` block.
+of variable ``!22`` should not flow "down" into the ``%exit`` block.
 
 .. _ccxx_frontend:
 
 C/C++ front-end specific debug information
 ==========================================
 
 The C and C++ front-ends represent information about the program in a
 format that is effectively identical to `DWARF <http://www.dwarfstd.org/>`_

llvm/include/llvm/AsmParser/LLParser.h

private:
     bool parseOptionalFunctionMetadata(Function &F);
 
     template <class FieldTy>
     bool parseMDField(LocTy Loc, StringRef Name, FieldTy &Result);
     template <class FieldTy> bool parseMDField(StringRef Name, FieldTy &Result);
     template <class ParserTy> bool parseMDFieldsImplBody(ParserTy ParseField);
     template <class ParserTy>
     bool parseMDFieldsImpl(ParserTy ParseField, LocTy &ClosingLoc);
-    bool parseSpecializedMDNode(MDNode *&N, bool IsDistinct = false);
+    bool parseSpecializedMDNode(MDNode *&N, bool IsDistinct = false,
+                                LocTy DistinctLoc = LocTy());
 
 #define HANDLE_SPECIALIZED_MDNODE_LEAF(CLASS)                                  \
   bool parse##CLASS(MDNode *&Result, bool IsDistinct);
 #include "llvm/IR/Metadata.def"
-    bool parseDIArgList(MDNode *&Result, bool IsDistinct,
+    bool parseDIArgList(Metadata *&Result, bool IsDistinct,
                         PerFunctionState *PFS);
 
     // Function Parsing.
     struct ArgInfo {
       LocTy Loc;
       Type *Ty;
       AttributeSet Attrs;
       std::string Name;

llvm/include/llvm/IR/DebugInfoMetadata.h

 #include <cstdint>
 #include <iterator>
 #include <type_traits>
 #include <vector>
 
 // Helper macros for defining get() overrides.
 #define DEFINE_MDNODE_GET_UNPACK_IMPL(...) __VA_ARGS__
 #define DEFINE_MDNODE_GET_UNPACK(ARGS) DEFINE_MDNODE_GET_UNPACK_IMPL ARGS
-#define DEFINE_MDNODE_GET_DISTINCT_TEMPORARY(CLASS, FORMAL, ARGS)              \
+#define DEFINE_MDNODE_GET(CLASS, FORMAL, ARGS)                                 \
+  static CLASS *get(LLVMContext &Context, DEFINE_MDNODE_GET_UNPACK(FORMAL)) {  \
+    return getImpl(Context, DEFINE_MDNODE_GET_UNPACK(ARGS), Uniqued);          \
+  }
+#define DEFINE_MDNODE_GET_IF_EXISTS(CLASS, FORMAL, ARGS)                       \
+  static CLASS *getIfExists(LLVMContext &Context,                              \
+                            DEFINE_MDNODE_GET_UNPACK(FORMAL)) {                \
+    return getImpl(Context, DEFINE_MDNODE_GET_UNPACK(ARGS), Uniqued,           \
+                   /* ShouldCreate */ false);                                  \
+  }
+#define DEFINE_MDNODE_GET_DISTINCT(CLASS, FORMAL, ARGS)                        \
   static CLASS *getDistinct(LLVMContext &Context,                              \
                             DEFINE_MDNODE_GET_UNPACK(FORMAL)) {                \
     return getImpl(Context, DEFINE_MDNODE_GET_UNPACK(ARGS), Distinct);         \
-  }                                                                            \
+  }
+#define DEFINE_MDNODE_GET_TEMPORARY(CLASS, FORMAL, ARGS)                       \
   static Temp##CLASS getTemporary(LLVMContext &Context,                        \
                                   DEFINE_MDNODE_GET_UNPACK(FORMAL)) {          \
     return Temp##CLASS(                                                        \
         getImpl(Context, DEFINE_MDNODE_GET_UNPACK(ARGS), Temporary));          \
   }
-#define DEFINE_MDNODE_GET(CLASS, FORMAL, ARGS)                                 \
-  static CLASS *get(LLVMContext &Context, DEFINE_MDNODE_GET_UNPACK(FORMAL)) {  \
-    return getImpl(Context, DEFINE_MDNODE_GET_UNPACK(ARGS), Uniqued);          \
-  }                                                                            \
-  static CLASS *getIfExists(LLVMContext &Context,                              \
-                            DEFINE_MDNODE_GET_UNPACK(FORMAL)) {                \
-    return getImpl(Context, DEFINE_MDNODE_GET_UNPACK(ARGS), Uniqued,           \
-                   /* ShouldCreate */ false);                                  \
-  }                                                                            \
-  DEFINE_MDNODE_GET_DISTINCT_TEMPORARY(CLASS, FORMAL, ARGS)
+#define DEFINE_ALL_MDNODE_GET_METHODS(CLASS, FORMAL, ARGS)                     \
+  DEFINE_MDNODE_GET(CLASS, FORMAL, ARGS)                                       \
+  DEFINE_MDNODE_GET_IF_EXISTS(CLASS, FORMAL, ARGS)                             \
+  DEFINE_MDNODE_GET_DISTINCT(CLASS, FORMAL, ARGS)                              \
+  DEFINE_MDNODE_GET_TEMPORARY(CLASS, FORMAL, ARGS)
+#define DEFINE_ALWAYS_DISTINCT_MDNODE_GET_METHODS(CLASS, FORMAL, ARGS)         \
+  DEFINE_MDNODE_GET_DISTINCT(CLASS, FORMAL, ARGS)                              \
+  DEFINE_MDNODE_GET_TEMPORARY(CLASS, FORMAL, ARGS)
+#define DEFINE_ALWAYS_UNIQUED_MDNODE_GET_METHODS(CLASS, FORMAL, ARGS)          \
+  DEFINE_MDNODE_GET(CLASS, FORMAL, ARGS)                                       \
+  DEFINE_MDNODE_GET_IF_EXISTS(CLASS, FORMAL, ARGS)                             \
+  DEFINE_MDNODE_GET_TEMPORARY(CLASS, FORMAL, ARGS)
 
 namespace llvm {
 
 extern cl::opt<bool> EnableFSDiscriminator;
 
 class DITypeRefArray {
   const MDTuple *N = nullptr;
 
class GenericDINode : public DINode {
   TempGenericDINode cloneImpl() const {
     return getTemporary(getContext(), getTag(), getHeader(),
                         SmallVector<Metadata *, 4>(dwarf_operands()));
   }
 
 public:
   unsigned getHash() const { return SubclassData32; }
 
-  DEFINE_MDNODE_GET(GenericDINode,
+  DEFINE_ALL_MDNODE_GET_METHODS(GenericDINode,
                                 (unsigned Tag, StringRef Header,
                                  ArrayRef<Metadata *> DwarfOps),
                                 (Tag, Header, DwarfOps))
-  DEFINE_MDNODE_GET(GenericDINode,
+  DEFINE_ALL_MDNODE_GET_METHODS(GenericDINode,
                                 (unsigned Tag, MDString *Header,
                                  ArrayRef<Metadata *> DwarfOps),
                                 (Tag, Header, DwarfOps))
 
   /// Return a (temporary) clone of this.
   TempGenericDINode clone() const { return cloneImpl(); }
 
   dwarf::Tag getTag() const { return (dwarf::Tag)SubclassData16; }
   StringRef getHeader() const { return getStringOperand(0); }
   MDString *getRawHeader() const { return getOperandAs<MDString>(0); }
 
static DISubrange *getImpl(LLVMContext &Context, Metadata *CountNode,
                              bool ShouldCreate = true);
 
   TempDISubrange cloneImpl() const {
     return getTemporary(getContext(), getRawCountNode(), getRawLowerBound(),
                         getRawUpperBound(), getRawStride());
   }
 
 public:
-  DEFINE_MDNODE_GET(DISubrange, (int64_t Count, int64_t LowerBound = 0),
+  DEFINE_ALL_MDNODE_GET_METHODS(DISubrange,
+                                (int64_t Count, int64_t LowerBound = 0),
                                 (Count, LowerBound))
 
-  DEFINE_MDNODE_GET(DISubrange, (Metadata * CountNode, int64_t LowerBound = 0),
+  DEFINE_ALL_MDNODE_GET_METHODS(DISubrange,
+                                (Metadata * CountNode, int64_t LowerBound = 0),
                                 (CountNode, LowerBound))
 
-  DEFINE_MDNODE_GET(DISubrange,
+  DEFINE_ALL_MDNODE_GET_METHODS(DISubrange,
                                 (Metadata * CountNode, Metadata *LowerBound,
                                  Metadata *UpperBound, Metadata *Stride),
                                 (CountNode, LowerBound, UpperBound, Stride))
 
   TempDISubrange clone() const { return cloneImpl(); }
 
-  Metadata *getRawCountNode() const { return getOperand(0).get(); }
+  Metadata *getRawCountNode() const {
+    return getOperand(0).get();
+  }
 
   Metadata *getRawLowerBound() const { return getOperand(1).get(); }
 
   Metadata *getRawUpperBound() const { return getOperand(2).get(); }
 
   Metadata *getRawStride() const { return getOperand(3).get(); }
 
   typedef PointerUnion<ConstantInt *, DIVariable *, DIExpression *> BoundType;
static DIGenericSubrange *getImpl(LLVMContext &Context, Metadata *CountNode,
                                     bool ShouldCreate = true);
 
   TempDIGenericSubrange cloneImpl() const {
     return getTemporary(getContext(), getRawCountNode(), getRawLowerBound(),
                         getRawUpperBound(), getRawStride());
   }
 
 public:
-  DEFINE_MDNODE_GET(DIGenericSubrange,
+  DEFINE_ALL_MDNODE_GET_METHODS(DIGenericSubrange,
                                 (Metadata * CountNode, Metadata *LowerBound,
                                  Metadata *UpperBound, Metadata *Stride),
                                 (CountNode, LowerBound, UpperBound, Stride))
 
   TempDIGenericSubrange clone() const { return cloneImpl(); }
 
   Metadata *getRawCountNode() const { return getOperand(0).get(); }
   Metadata *getRawLowerBound() const { return getOperand(1).get(); }
   Metadata *getRawUpperBound() const { return getOperand(2).get(); }
   Metadata *getRawStride() const { return getOperand(3).get(); }
 
static DIEnumerator *getImpl(LLVMContext &Context, const APInt &Value,
                                bool IsUnsigned, MDString *Name,
                                StorageType Storage, bool ShouldCreate = true);
 
   TempDIEnumerator cloneImpl() const {
     return getTemporary(getContext(), getValue(), isUnsigned(), getName());
   }
 
 public:
-  DEFINE_MDNODE_GET(DIEnumerator,
-                    (int64_t Value, bool IsUnsigned, StringRef Name),
-                    (APInt(64, Value, !IsUnsigned), IsUnsigned, Name))
-  DEFINE_MDNODE_GET(DIEnumerator,
-                    (int64_t Value, bool IsUnsigned, MDString *Name),
-                    (APInt(64, Value, !IsUnsigned), IsUnsigned, Name))
-  DEFINE_MDNODE_GET(DIEnumerator,
+  DEFINE_ALL_MDNODE_GET_METHODS(
+      DIEnumerator, (int64_t Value, bool IsUnsigned, StringRef Name),
+  DEFINE_ALL_MDNODE_GET_METHODS(
+      DIEnumerator, (int64_t Value, bool IsUnsigned, MDString *Name),
+  DEFINE_ALL_MDNODE_GET_METHODS(DIEnumerator,
                                 (APInt Value, bool IsUnsigned, StringRef Name),
                                 (Value, IsUnsigned, Name))
-  DEFINE_MDNODE_GET(DIEnumerator,
+  DEFINE_ALL_MDNODE_GET_METHODS(DIEnumerator,
                                 (APInt Value, bool IsUnsigned, MDString *Name),
                                 (Value, IsUnsigned, Name))
 
   TempDIEnumerator clone() const { return cloneImpl(); }
 
   const APInt &getValue() const { return Value; }
   bool isUnsigned() const { return SubclassData32; }
   StringRef getName() const { return getStringOperand(0); }
 
   MDString *getRawName() const { return getOperandAs<MDString>(0); }
enum ChecksumKind {
     // encoding is reserved.
     CSK_MD5 = 1,
     CSK_SHA1 = 2,
     CSK_SHA256 = 3,
     CSK_Last = CSK_SHA256 // Should be last enumeration.
   };
 
   /// A single checksum, represented by a \a Kind and a \a Value (a string).
-  template <typename T> struct ChecksumInfo {
+  template <typename T>
+  struct ChecksumInfo {
     /// The kind of checksum which \a Value encodes.
     ChecksumKind Kind;
     /// The string value of the checksum.
     T Value;
 
-    ChecksumInfo(ChecksumKind Kind, T Value) : Kind(Kind), Value(Value) {}
+    ChecksumInfo(ChecksumKind Kind, T Value) : Kind(Kind), Value(Value) { }
     ~ChecksumInfo() = default;
     bool operator==(const ChecksumInfo<T> &X) const {
       return Kind == X.Kind && Value == X.Value;
     }
     bool operator!=(const ChecksumInfo<T> &X) const { return !(*this == X); }
     StringRef getKindAsString() const { return getChecksumKindAsString(Kind); }
   };
 
 private:
   Optional<ChecksumInfo<MDString *>> Checksum;
   Optional<MDString *> Source;
 
   DIFile(LLVMContext &C, StorageType Storage,
          Optional<ChecksumInfo<MDString *>> CS, Optional<MDString *> Src,
          ArrayRef<Metadata *> Ops)
       : DIScope(C, DIFileKind, Storage, dwarf::DW_TAG_file_type, Ops),
         Checksum(CS), Source(Src) {}
   ~DIFile() = default;
 
   static DIFile *getImpl(LLVMContext &Context, StringRef Filename,
                          StringRef Directory,
                          Optional<ChecksumInfo<StringRef>> CS,
-                         Optional<StringRef> Source, StorageType Storage,
-                         bool ShouldCreate = true) {
+                         Optional<StringRef> Source,
+                         StorageType Storage, bool ShouldCreate = true) {
     Optional<ChecksumInfo<MDString *>> MDChecksum;
     if (CS)
       MDChecksum.emplace(CS->Kind, getCanonicalMDString(Context, CS->Value));
-    return getImpl(
-        Context, getCanonicalMDString(Context, Filename),
+    return getImpl(Context, getCanonicalMDString(Context, Filename),
                    getCanonicalMDString(Context, Directory), MDChecksum,
-        Source ? Optional<MDString *>(getCanonicalMDString(Context, *Source))
-               : None,
+                   Source ? Optional<MDString *>(getCanonicalMDString(Context, *Source)) : None,
                    Storage, ShouldCreate);
   }
   static DIFile *getImpl(LLVMContext &Context, MDString *Filename,
                          MDString *Directory,
                          Optional<ChecksumInfo<MDString *>> CS,
                          Optional<MDString *> Source, StorageType Storage,
                          bool ShouldCreate = true);
 
   TempDIFile cloneImpl() const {
     return getTemporary(getContext(), getFilename(), getDirectory(),
                         getChecksum(), getSource());
   }
 
 public:
-  DEFINE_MDNODE_GET(DIFile,
+  DEFINE_ALL_MDNODE_GET_METHODS(DIFile,
                                 (StringRef Filename, StringRef Directory,
                                  Optional<ChecksumInfo<StringRef>> CS = None,
                                  Optional<StringRef> Source = None),
                                 (Filename, Directory, CS, Source))
-  DEFINE_MDNODE_GET(DIFile,
+  DEFINE_ALL_MDNODE_GET_METHODS(DIFile,
                                 (MDString * Filename, MDString *Directory,
                                  Optional<ChecksumInfo<MDString *>> CS = None,
                                  Optional<MDString *> Source = None),
                                 (Filename, Directory, CS, Source))
 
   TempDIFile clone() const { return cloneImpl(); }
 
   StringRef getFilename() const { return getStringOperand(0); }
   StringRef getDirectory() const { return getStringOperand(1); }
   Optional<ChecksumInfo<StringRef>> getChecksum() const {
     Optional<ChecksumInfo<StringRef>> StringRefChecksum;
     if (Checksum)
public:
   uint32_t getAlignInBits() const { return AlignInBits; }
   uint32_t getAlignInBytes() const { return getAlignInBits() / CHAR_BIT; }
   uint64_t getOffsetInBits() const { return OffsetInBits; }
   DIFlags getFlags() const { return Flags; }
 
   DIScope *getScope() const { return cast_or_null<DIScope>(getRawScope()); }
   StringRef getName() const { return getStringOperand(2); }
 
+
   Metadata *getRawScope() const { return getOperand(1); }
   MDString *getRawName() const { return getOperandAs<MDString>(2); }
 
   /// Returns a new temporary DIType with updated Flags
   TempDIType cloneWithFlags(DIFlags NewFlags) const {
     auto NewTy = clone();
     NewTy->Flags = NewFlags;
     return NewTy;
static DIBasicType *getImpl(LLVMContext &Context, unsigned Tag,
                               bool ShouldCreate = true);
 
   TempDIBasicType cloneImpl() const {
     return getTemporary(getContext(), getTag(), getName(), getSizeInBits(),
                         getAlignInBits(), getEncoding(), getFlags());
   }
 
 public:
-  DEFINE_MDNODE_GET(DIBasicType, (unsigned Tag, StringRef Name),
+  DEFINE_ALL_MDNODE_GET_METHODS(DIBasicType, (unsigned Tag, StringRef Name),
                                 (Tag, Name, 0, 0, 0, FlagZero))
-  DEFINE_MDNODE_GET(DIBasicType,
-                    (unsigned Tag, StringRef Name, uint64_t SizeInBits),
+  DEFINE_ALL_MDNODE_GET_METHODS(DIBasicType,
+                                (unsigned Tag, StringRef Name,
+                                 uint64_t SizeInBits),
                                 (Tag, Name, SizeInBits, 0, 0, FlagZero))
-  DEFINE_MDNODE_GET(DIBasicType,
-                    (unsigned Tag, MDString *Name, uint64_t SizeInBits),
+  DEFINE_ALL_MDNODE_GET_METHODS(DIBasicType,
+                                (unsigned Tag, MDString *Name,
+                                 uint64_t SizeInBits),
                                 (Tag, Name, SizeInBits, 0, 0, FlagZero))
-  DEFINE_MDNODE_GET(DIBasicType,
-                    (unsigned Tag, StringRef Name, uint64_t SizeInBits,
-                     uint32_t AlignInBits, unsigned Encoding, DIFlags Flags),
-                    (Tag, Name, SizeInBits, AlignInBits, Encoding, Flags))
-  DEFINE_MDNODE_GET(DIBasicType,
-                    (unsigned Tag, MDString *Name, uint64_t SizeInBits,
-                     uint32_t AlignInBits, unsigned Encoding, DIFlags Flags),
-                    (Tag, Name, SizeInBits, AlignInBits, Encoding, Flags))
+  DEFINE_ALL_MDNODE_GET_METHODS(DIBasicType,
+                                (unsigned Tag, StringRef Name,
+                                 uint64_t SizeInBits, uint32_t AlignInBits,
+                                 unsigned Encoding, DIFlags Flags),
+                                (Tag, Name, SizeInBits, AlignInBits, Encoding,
+                                 Flags))
+  DEFINE_ALL_MDNODE_GET_METHODS(DIBasicType,
+                                (unsigned Tag, MDString *Name,
+                                 uint64_t SizeInBits, uint32_t AlignInBits,
+                                 unsigned Encoding, DIFlags Flags),
+                                (Tag, Name, SizeInBits, AlignInBits, Encoding,
+                                 Flags))
 
   TempDIBasicType clone() const { return cloneImpl(); }
 
   unsigned getEncoding() const { return Encoding; }
 
   enum class Signedness { Signed, Unsigned };
 
   /// Return the signedness of this type, or None if this type is neither
class DIStringType : public DIType {
 
   TempDIStringType cloneImpl() const {
     return getTemporary(getContext(), getTag(), getRawName(),
                         getRawStringLength(), getRawStringLengthExp(),
                         getSizeInBits(), getAlignInBits(), getEncoding());
   }
 
 public:
-  DEFINE_MDNODE_GET(DIStringType,
-                    (unsigned Tag, StringRef Name, uint64_t SizeInBits,
-                     uint32_t AlignInBits),
-                    (Tag, Name, nullptr, nullptr, SizeInBits, AlignInBits, 0))
-  DEFINE_MDNODE_GET(DIStringType,
-                    (unsigned Tag, MDString *Name, Metadata *StringLength,
+  DEFINE_ALL_MDNODE_GET_METHODS(DIStringType,
+                                (unsigned Tag, StringRef Name,
+                                 uint64_t SizeInBits, uint32_t AlignInBits),
+                                (Tag, Name, nullptr, nullptr, SizeInBits,
+                                 AlignInBits, 0))
+  DEFINE_ALL_MDNODE_GET_METHODS(DIStringType,
+                                (unsigned Tag, MDString *Name,
+                                 Metadata *StringLength,
                                  Metadata *StringLengthExp, uint64_t SizeInBits,
                                  uint32_t AlignInBits, unsigned Encoding),
-                    (Tag, Name, StringLength, StringLengthExp, SizeInBits,
-                     AlignInBits, Encoding))
-  DEFINE_MDNODE_GET(DIStringType,
-                    (unsigned Tag, StringRef Name, Metadata *StringLength,
+                                (Tag, Name, StringLength, StringLengthExp,
+                                 SizeInBits, AlignInBits, Encoding))
+  DEFINE_ALL_MDNODE_GET_METHODS(DIStringType,
+                                (unsigned Tag, StringRef Name,
+                                 Metadata *StringLength,
                                  Metadata *StringLengthExp, uint64_t SizeInBits,
                                  uint32_t AlignInBits, unsigned Encoding),
-                    (Tag, Name, StringLength, StringLengthExp, SizeInBits,
-                     AlignInBits, Encoding))
+                                (Tag, Name, StringLength, StringLengthExp,
+                                 SizeInBits, AlignInBits, Encoding))
 
   TempDIStringType clone() const { return cloneImpl(); }
 
   static bool classof(const Metadata *MD) {
     return MD->getMetadataID() == DIStringTypeKind;
   }
 
   DIVariable *getStringLength() const {
class DIDerivedType : public DIType {
   TempDIDerivedType cloneImpl() const {
     return getTemporary(
         getContext(), getTag(), getName(), getFile(), getLine(), getScope(),
         getBaseType(), getSizeInBits(), getAlignInBits(), getOffsetInBits(),
         getDWARFAddressSpace(), getFlags(), getExtraData(), getAnnotations());
   }
 
 public:
-  DEFINE_MDNODE_GET(
+  DEFINE_ALL_MDNODE_GET_METHODS(
       DIDerivedType,
       (unsigned Tag, MDString *Name, Metadata *File, unsigned Line,
        Metadata *Scope, Metadata *BaseType, uint64_t SizeInBits,
        uint32_t AlignInBits, uint64_t OffsetInBits,
        Optional<unsigned> DWARFAddressSpace, DIFlags Flags,
        Metadata *ExtraData = nullptr, Metadata *Annotations = nullptr),
       (Tag, Name, File, Line, Scope, BaseType, SizeInBits, AlignInBits,
        OffsetInBits, DWARFAddressSpace, Flags, ExtraData, Annotations))
-  DEFINE_MDNODE_GET(DIDerivedType,
-                    (unsigned Tag, StringRef Name, DIFile *File, unsigned Line,
-                     DIScope *Scope, DIType *BaseType, uint64_t SizeInBits,
-                     uint32_t AlignInBits, uint64_t OffsetInBits,
-                     Optional<unsigned> DWARFAddressSpace, DIFlags Flags,
-                     Metadata *ExtraData = nullptr,
-                     DINodeArray Annotations = nullptr),
-                    (Tag, Name, File, Line, Scope, BaseType, SizeInBits,
-                     AlignInBits, OffsetInBits, DWARFAddressSpace, Flags,
-                     ExtraData, Annotations))
+  DEFINE_ALL_MDNODE_GET_METHODS(
+      DIDerivedType,
+       Metadata *ExtraData = nullptr, DINodeArray Annotations = nullptr),
+      (Tag, Name, File, Line, Scope, BaseType, SizeInBits, AlignInBits,
+       OffsetInBits, DWARFAddressSpace, Flags, ExtraData, Annotations))
 
   TempDIDerivedType clone() const { return cloneImpl(); }
 
   /// Get the base type this is derived from.
   DIType *getBaseType() const { return cast_or_null<DIType>(getRawBaseType()); }
   Metadata *getRawBaseType() const { return getOperand(3); }
 
   /// \returns The DWARF address space of the memory pointed to or referenced by
DICompositeType(LLVMContext &C, StorageType Storage, unsigned Tag,
                   ArrayRef<Metadata *> Ops)
       : DIType(C, DICompositeTypeKind, Storage, Tag, Line, SizeInBits,
                AlignInBits, OffsetInBits, Flags, Ops),
         RuntimeLang(RuntimeLang) {}
   ~DICompositeType() = default;
 
   /// Change fields in place.
   void mutate(unsigned Tag, unsigned Line, unsigned RuntimeLang,
-              uint64_t SizeInBits, uint32_t AlignInBits, uint64_t OffsetInBits,
-              DIFlags Flags) {
+              uint64_t SizeInBits, uint32_t AlignInBits,
+              uint64_t OffsetInBits, DIFlags Flags) {
     assert(isDistinct() && "Only distinct nodes can mutate");
     assert(getRawIdentifier() && "Only ODR-uniqued nodes should mutate");
     this->RuntimeLang = RuntimeLang;
     DIType::mutate(Tag, Line, SizeInBits, AlignInBits, OffsetInBits, Flags);
   }
 
   static DICompositeType *
   getImpl(LLVMContext &Context, unsigned Tag, StringRef Name, Metadata *File,
return getTemporary(
         getBaseType(), getSizeInBits(), getAlignInBits(), getOffsetInBits(),
         getFlags(), getElements(), getRuntimeLang(), getVTableHolder(),
         getTemplateParams(), getIdentifier(), getDiscriminator(),
         getRawDataLocation(), getRawAssociated(), getRawAllocated(),
         getRawRank(), getAnnotations());
   }
 
 public:
-  DEFINE_MDNODE_GET(
+  DEFINE_ALL_MDNODE_GET_METHODS(
       DICompositeType,
       (unsigned Tag, StringRef Name, DIFile *File, unsigned Line,
        DIScope *Scope, DIType *BaseType, uint64_t SizeInBits,
        uint32_t AlignInBits, uint64_t OffsetInBits, DIFlags Flags,
        DINodeArray Elements, unsigned RuntimeLang, DIType *VTableHolder,
        DITemplateParameterArray TemplateParams = nullptr,
        StringRef Identifier = "", DIDerivedType *Discriminator = nullptr,
        Metadata *DataLocation = nullptr, Metadata *Associated = nullptr,
        Metadata *Allocated = nullptr, Metadata *Rank = nullptr,
        DINodeArray Annotations = nullptr),
       (Tag, Name, File, Line, Scope, BaseType, SizeInBits, AlignInBits,
        OffsetInBits, Flags, Elements, RuntimeLang, VTableHolder, TemplateParams,
        Identifier, Discriminator, DataLocation, Associated, Allocated, Rank,
        Annotations))
-  DEFINE_MDNODE_GET(
+  DEFINE_ALL_MDNODE_GET_METHODS(
       DICompositeType,
       (unsigned Tag, MDString *Name, Metadata *File, unsigned Line,
        Metadata *Scope, Metadata *BaseType, uint64_t SizeInBits,
        uint32_t AlignInBits, uint64_t OffsetInBits, DIFlags Flags,
        Metadata *Elements, unsigned RuntimeLang, Metadata *VTableHolder,
        Metadata *TemplateParams = nullptr, MDString *Identifier = nullptr,
        Metadata *Discriminator = nullptr, Metadata *DataLocation = nullptr,
        Metadata *Associated = nullptr, Metadata *Allocated = nullptr,
public:
   unsigned getRuntimeLang() const { return RuntimeLang; }
 
   Metadata *getRawBaseType() const { return getOperand(3); }
   Metadata *getRawElements() const { return getOperand(4); }
   Metadata *getRawVTableHolder() const { return getOperand(5); }
   Metadata *getRawTemplateParams() const { return getOperand(6); }
   MDString *getRawIdentifier() const { return getOperandAs<MDString>(7); }
   Metadata *getRawDiscriminator() const { return getOperand(8); }
-  DIDerivedType *getDiscriminator() const {
-    return getOperandAs<DIDerivedType>(8);
-  }
+  DIDerivedType *getDiscriminator() const { return getOperandAs<DIDerivedType>(8); }
   Metadata *getRawDataLocation() const { return getOperand(9); }
   DIVariable *getDataLocation() const {
     return dyn_cast_or_null<DIVariable>(getRawDataLocation());
   }
   DIExpression *getDataLocationExp() const {
     return dyn_cast_or_null<DIExpression>(getRawDataLocation());
   }
   Metadata *getRawAssociated() const { return getOperand(10); }
static DISubroutineType *getImpl(LLVMContext &Context, DIFlags Flags,
                                    StorageType Storage,
                                    bool ShouldCreate = true);
 
   TempDISubroutineType cloneImpl() const {
     return getTemporary(getContext(), getFlags(), getCC(), getTypeArray());
   }
 
 public:
-  DEFINE_MDNODE_GET(DISubroutineType,
-                    (DIFlags Flags, uint8_t CC, DITypeRefArray TypeArray),
+  DEFINE_ALL_MDNODE_GET_METHODS(DISubroutineType,
+                                (DIFlags Flags, uint8_t CC,
+                                 DITypeRefArray TypeArray),
                                 (Flags, CC, TypeArray))
-  DEFINE_MDNODE_GET(DISubroutineType,
-                    (DIFlags Flags, uint8_t CC, Metadata *TypeArray),
+  DEFINE_ALL_MDNODE_GET_METHODS(DISubroutineType,
+                                (DIFlags Flags, uint8_t CC,
+                                 Metadata *TypeArray),
                                 (Flags, CC, TypeArray))
 
   TempDISubroutineType clone() const { return cloneImpl(); }
 
   uint8_t getCC() const { return CC; }
 
   DITypeRefArray getTypeArray() const {
     return cast_or_null<MDTuple>(getRawTypeArray());
   }
return getTemporary(
         getSplitDebugInlining(), getDebugInfoForProfiling(), getNameTableKind(),
         getRangesBaseAddress(), getSysRoot(), getSDK());
   }
 
 public:
   static void get() = delete;
   static void getIfExists() = delete;
 
-  DEFINE_MDNODE_GET_DISTINCT_TEMPORARY(
+  DEFINE_ALWAYS_DISTINCT_MDNODE_GET_METHODS(
       DICompileUnit,
       (unsigned SourceLanguage, DIFile *File, StringRef Producer,
        bool IsOptimized, StringRef Flags, unsigned RuntimeVersion,
        StringRef SplitDebugFilename, DebugEmissionKind EmissionKind,
        DICompositeTypeArray EnumTypes, DIScopeArray RetainedTypes,
        DIGlobalVariableExpressionArray GlobalVariables,
        DIImportedEntityArray ImportedEntities, DIMacroNodeArray Macros,
        uint64_t DWOId, bool SplitDebugInlining, bool DebugInfoForProfiling,
        DebugNameTableKind NameTableKind, bool RangesBaseAddress,
        StringRef SysRoot, StringRef SDK),
       (SourceLanguage, File, Producer, IsOptimized, Flags, RuntimeVersion,
        SplitDebugFilename, EmissionKind, EnumTypes, RetainedTypes,
        GlobalVariables, ImportedEntities, Macros, DWOId, SplitDebugInlining,
        DebugInfoForProfiling, (unsigned)NameTableKind, RangesBaseAddress,
        SysRoot, SDK))
-  DEFINE_MDNODE_GET_DISTINCT_TEMPORARY(
+  DEFINE_ALWAYS_DISTINCT_MDNODE_GET_METHODS(
       DICompileUnit,
       (unsigned SourceLanguage, Metadata *File, MDString *Producer,
        bool IsOptimized, MDString *Flags, unsigned RuntimeVersion,
        MDString *SplitDebugFilename, unsigned EmissionKind, Metadata *EnumTypes,
        Metadata *RetainedTypes, Metadata *GlobalVariables,
        Metadata *ImportedEntities, Metadata *Macros, uint64_t DWOId,
        bool SplitDebugInlining, bool DebugInfoForProfiling,
        unsigned NameTableKind, bool RangesBaseAddress, MDString *SysRoot,
public:
   ///
   /// If this \a isUniqued() and not \a isResolved(), it will be RAUW'ed and
   /// deleted on a uniquing collision.  In practice, uniquing collisions on \a
   /// DICompileUnit should be fairly rare.
   /// @{
   void replaceEnumTypes(DICompositeTypeArray N) {
     replaceOperandWith(4, N.get());
   }
-  void replaceRetainedTypes(DITypeArray N) { replaceOperandWith(5, N.get()); }
+  void replaceRetainedTypes(DITypeArray N) {
+    replaceOperandWith(5, N.get());
+  }
   void replaceGlobalVariables(DIGlobalVariableExpressionArray N) {
     replaceOperandWith(6, N.get());
   }
   void replaceImportedEntities(DIImportedEntityArray N) {
     replaceOperandWith(7, N.get());
   }
   void replaceMacros(DIMacroNodeArray N) { replaceOperandWith(8, N.get()); }
   /// @}
TempDILocation cloneImpl() const {
     return getTemporary(getContext(), getLine(), getColumn(), getRawScope(),
                         getRawInlinedAt(), isImplicitCode());
   }
 
 public:
   // Disallow replacing operands.
   void replaceOperandWith(unsigned I, Metadata *New) = delete;
 
-  DEFINE_MDNODE_GET(DILocation,
-                    (unsigned Line, unsigned Column, Metadata *Scope,
-                     Metadata *InlinedAt = nullptr, bool ImplicitCode = false),
+  DEFINE_ALL_MDNODE_GET_METHODS(DILocation,
+                                (unsigned Line, unsigned Column,
+                                 Metadata *Scope, Metadata *InlinedAt = nullptr,
+                                 bool ImplicitCode = false),
                                 (Line, Column, Scope, InlinedAt, ImplicitCode))
-  DEFINE_MDNODE_GET(DILocation,
-                    (unsigned Line, unsigned Column, DILocalScope *Scope,
+  DEFINE_ALL_MDNODE_GET_METHODS(DILocation,
+                                (unsigned Line, unsigned Column,
+                                 DILocalScope *Scope,
                                  DILocation *InlinedAt = nullptr,
                                  bool ImplicitCode = false),
                                 (Line, Column, Scope, InlinedAt, ImplicitCode))
 
   /// Return a (temporary) clone of this.
   TempDILocation clone() const { return cloneImpl(); }
 
   unsigned getLine() const { return SubclassData32; }
   unsigned getColumn() const { return SubclassData16; }
   DILocalScope *getScope() const { return cast<DILocalScope>(getRawScope()); }
 
public:
 
   /// Returns a new DILocation with updated \p Discriminator.
   inline const DILocation *cloneWithDiscriminator(unsigned Discriminator) const;
 
   /// Returns a new DILocation with updated base discriminator \p BD. Only the
   /// base discriminator is set in the new DILocation, the other encoded values
   /// are elided.
   /// If the discriminator cannot be encoded, the function returns None.
-  inline Optional<const DILocation *>
-  cloneWithBaseDiscriminator(unsigned BD) const;
+  inline Optional<const DILocation *> cloneWithBaseDiscriminator(unsigned BD) const;
 
   /// Returns the duplication factor stored in the discriminator, or 1 if no
   /// duplication factor (or 0) is encoded.
   inline unsigned getDuplicationFactor() const;
 
   /// Returns the copy identifier stored in the discriminator.
   inline unsigned getCopyIdentifier() const;
 
   /// Returns the base discriminator stored in the discriminator.
   inline unsigned getBaseDiscriminator() const;
 
   /// Returns a new DILocation with duplication factor \p DF * current
   /// duplication factor encoded in the discriminator. The current duplication
   /// factor is as defined by getDuplicationFactor().
   /// Returns None if encoding failed.
-  inline Optional<const DILocation *>
-  cloneByMultiplyingDuplicationFactor(unsigned DF) const;
+  inline Optional<const DILocation *> cloneByMultiplyingDuplicationFactor(unsigned DF) const;
 
   /// When two instructions are combined into a single instruction we also
   /// need to combine the original locations into a single location.
   ///
   /// When the locations are the same we can use either location. When they
   /// differ, we need a third location which is distinct from either. If they
   /// have the same file/line but have a different discriminator we could
   /// create a location with a new discriminator. If they are from different
   /// files/lines the location is ambiguous and can't be represented in a line
   /// entry. In this case, if \p GenerateLocation is true, we will set the
   /// merged debug location as line 0 of the nearest common scope where the two
   /// locations are inlined from.
   ///
   /// \p GenerateLocation: Whether the merged location can be generated when
   /// \p LocA and \p LocB differ.
   static const DILocation *getMergedLocation(const DILocation *LocA,
                                              const DILocation *LocB);
 
   /// Try to combine the vector of locations passed as input in a single one.
   /// This function applies getMergedLocation() repeatedly left-to-right.
   ///
   /// \p Locs: The locations to be merged.
-  static const DILocation *
-  getMergedLocations(ArrayRef<const DILocation *> Locs);
+  static
+  const DILocation *getMergedLocations(ArrayRef<const DILocation *> Locs);
 
   /// Return the masked discriminator value for an input discrimnator value D
   /// (i.e. zero out the (B+1)-th and above bits for D (B is 0-base).
   // Example: an input of (0x1FF, 7) returns 0xFF.
   static unsigned getMaskedDiscriminator(unsigned D, unsigned B) {
     return (D & getN1Bits(B));
   }
 
   /// Return the bits used for base discriminators.
   static unsigned getBaseDiscriminatorBits() { return getBaseFSBitEnd(); }
 
   /// Returns the base discriminator for a given encoded discriminator \p D.
   static unsigned
   getBaseDiscriminatorFromDiscriminator(unsigned D,
                                         bool IsFSDiscriminator = false) {
     if (IsFSDiscriminator)
       return getMaskedDiscriminator(D, getBaseDiscriminatorBits());
     return getUnsignedFromPrefixEncoding(D);
   }
 
   /// Raw encoding of the discriminator. APIs such as cloneWithDuplicationFactor
   /// have certain special case behavior (e.g. treating empty duplication factor
   /// as the value '1').
-  /// This API, in conjunction with cloneWithDiscriminator, may be used to
-  /// encode the raw values provided.
-  ///
-  /// \p BD: base discriminator
-  /// \p DF: duplication factor
+  /// This API, in conjunction with cloneWithDiscriminator, may be used to encode
+  /// the raw values provided. \p BD: base discriminator \p DF: duplication factor
   /// \p CI: copy index
-  ///
   /// The return is None if the values cannot be encoded in 32 bits - for
-  /// example, values for BD or DF larger than 12 bits. Otherwise, the return is
-  /// the encoded value.
-  static Optional<unsigned> encodeDiscriminator(unsigned BD, unsigned DF,
-                                                unsigned CI);
+  /// example, values for BD or DF larger than 12 bits. Otherwise, the return
+  /// is the encoded value.
+  static Optional<unsigned> encodeDiscriminator(unsigned BD, unsigned DF, unsigned CI);
 
   /// Raw decoder for values in an encoded discriminator D.
   static void decodeDiscriminator(unsigned D, unsigned &BD, unsigned &DF,
                                   unsigned &CI);
 
   /// Returns the duplication factor for a given encoded discriminator \p D, or
   /// 1 if no value or 0 is encoded.
   static unsigned getDuplicationFactorFromDiscriminator(unsigned D) {
     if (EnableFSDiscriminator)
       return 1;
     D = getNextComponentInDiscriminator(D);
     unsigned Ret = getUnsignedFromPrefixEncoding(D);
     if (Ret == 0)
       return 1;
     return Ret;
   }
 
   /// Returns the copy identifier for a given encoded discriminator \p D.
   static unsigned getCopyIdentifierFromDiscriminator(unsigned D) {
-    return getUnsignedFromPrefixEncoding(
-        getNextComponentInDiscriminator(getNextComponentInDiscriminator(D)));
+    return getUnsignedFromPrefixEncoding(getNextComponentInDiscriminator(
+        getNextComponentInDiscriminator(D)));
   }
 
+
   Metadata *getRawScope() const { return getOperand(0); }
   Metadata *getRawInlinedAt() const {
     if (getNumOperands() == 2)
       return getOperand(1);
     return nullptr;
   }
 
   static bool classof(const Metadata *MD) {
static DISPFlags splitFlags(DISPFlags Flags,
                               SmallVectorImpl<DISPFlags> &SplitFlags);
 
   // Helper for converting old bitfields to new flags word.
   static DISPFlags toSPFlags(bool IsLocalToUnit, bool IsDefinition,
                              bool IsOptimized,
                              unsigned Virtuality = SPFlagNonvirtual,
                              bool IsMainSubprogram = false) {
     // We're assuming virtuality is the low-order field.
-    static_assert(int(SPFlagVirtual) == int(dwarf::DW_VIRTUALITY_virtual) &&
-                      int(SPFlagPureVirtual) ==
-                          int(dwarf::DW_VIRTUALITY_pure_virtual),
-                  "Virtuality constant mismatch");
+    static_assert(
+        int(SPFlagVirtual) == int(dwarf::DW_VIRTUALITY_virtual) &&
+            int(SPFlagPureVirtual) == int(dwarf::DW_VIRTUALITY_pure_virtual),
     return static_cast<DISPFlags>(
         (Virtuality & SPFlagVirtuality) |
         (IsLocalToUnit ? SPFlagLocalToUnit : SPFlagZero) |
         (IsDefinition ? SPFlagDefinition : SPFlagZero) |
         (IsOptimized ? SPFlagOptimized : SPFlagZero) |
         (IsMainSubprogram ? SPFlagMainSubprogram : SPFlagZero));
   }
 
getImpl(LLVMContext &Context, DIScope *Scope, StringRef Name,
           bool ShouldCreate = true) {
     return getImpl(Context, Scope, getCanonicalMDString(Context, Name),
                    getCanonicalMDString(Context, LinkageName), File, Line, Type,
                    ScopeLine, ContainingType, VirtualIndex, ThisAdjustment,
                    Flags, SPFlags, Unit, TemplateParams.get(), Declaration,
                    RetainedNodes.get(), ThrownTypes.get(), Annotations.get(),
                    Storage, ShouldCreate);
   }
-  static DISubprogram *
-  getImpl(LLVMContext &Context, Metadata *Scope, MDString *Name,
-          MDString *LinkageName, Metadata *File, unsigned Line, Metadata *Type,
-          unsigned ScopeLine, Metadata *ContainingType, unsigned VirtualIndex,
-          int ThisAdjustment, DIFlags Flags, DISPFlags SPFlags, Metadata *Unit,
+  static DISubprogram *getImpl(LLVMContext &Context, Metadata *Scope,
+                               MDString *Name, MDString *LinkageName,
+                               Metadata *File, unsigned Line, Metadata *Type,
+                               unsigned ScopeLine, Metadata *ContainingType,
+                               unsigned VirtualIndex, int ThisAdjustment,
+                               DIFlags Flags, DISPFlags SPFlags, Metadata *Unit,
                                Metadata *TemplateParams, Metadata *Declaration,
-          Metadata *RetainedNodes, Metadata *ThrownTypes, Metadata *Annotations,
-          StorageType Storage, bool ShouldCreate = true);
+                               Metadata *RetainedNodes, Metadata *ThrownTypes,
+                               Metadata *Annotations, StorageType Storage,
+                               bool ShouldCreate = true);
 
   TempDISubprogram cloneImpl() const {
     return getTemporary(getContext(), getScope(), getName(), getLinkageName(),
                         getFile(), getLine(), getType(), getScopeLine(),
                         getContainingType(), getVirtualIndex(),
                         getThisAdjustment(), getFlags(), getSPFlags(),
                         getUnit(), getTemplateParams(), getDeclaration(),
                         getRetainedNodes(), getThrownTypes(), getAnnotations());
   }
 
 public:
-  DEFINE_MDNODE_GET(
+  DEFINE_ALL_MDNODE_GET_METHODS(
       DISubprogram,
       (DIScope * Scope, StringRef Name, StringRef LinkageName, DIFile *File,
        unsigned Line, DISubroutineType *Type, unsigned ScopeLine,
        DIType *ContainingType, unsigned VirtualIndex, int ThisAdjustment,
        DIFlags Flags, DISPFlags SPFlags, DICompileUnit *Unit,
        DITemplateParameterArray TemplateParams = nullptr,
        DISubprogram *Declaration = nullptr, DINodeArray RetainedNodes = nullptr,
        DITypeArray ThrownTypes = nullptr, DINodeArray Annotations = nullptr),
       (Scope, Name, LinkageName, File, Line, Type, ScopeLine, ContainingType,
        VirtualIndex, ThisAdjustment, Flags, SPFlags, Unit, TemplateParams,
        Declaration, RetainedNodes, ThrownTypes, Annotations))
 
-  DEFINE_MDNODE_GET(
+  DEFINE_ALL_MDNODE_GET_METHODS(
       DISubprogram,
       (Metadata * Scope, MDString *Name, MDString *LinkageName, Metadata *File,
        unsigned Line, Metadata *Type, unsigned ScopeLine,
        Metadata *ContainingType, unsigned VirtualIndex, int ThisAdjustment,
        DIFlags Flags, DISPFlags SPFlags, Metadata *Unit,
        Metadata *TemplateParams = nullptr, Metadata *Declaration = nullptr,
        Metadata *RetainedNodes = nullptr, Metadata *ThrownTypes = nullptr,
        Metadata *Annotations = nullptr),
public:
   }
 
 public:
   unsigned getLine() const { return Line; }
   unsigned getVirtuality() const { return getSPFlags() & SPFlagVirtuality; }
   unsigned getVirtualIndex() const { return VirtualIndex; }
   int getThisAdjustment() const { return ThisAdjustment; }
   unsigned getScopeLine() const { return ScopeLine; }
-  void setScopeLine(unsigned L) {
-    assert(isDistinct());
-    ScopeLine = L;
-  }
+  void setScopeLine(unsigned L) { assert(isDistinct()); ScopeLine = L; }
   DIFlags getFlags() const { return Flags; }
   DISPFlags getSPFlags() const { return SPFlags; }
   bool isLocalToUnit() const { return getSPFlags() & SPFlagLocalToUnit; }
   bool isDefinition() const { return getSPFlags() & SPFlagDefinition; }
   bool isOptimized() const { return getSPFlags() & SPFlagOptimized; }
   bool isMainSubprogram() const { return getSPFlags() & SPFlagMainSubprogram; }
 
   bool isArtificial() const { return getFlags() & FlagArtificial; }
static DILexicalBlock *getImpl(LLVMContext &Context, Metadata *Scope,
                                  StorageType Storage, bool ShouldCreate = true);
 
   TempDILexicalBlock cloneImpl() const {
     return getTemporary(getContext(), getScope(), getFile(), getLine(),
                         getColumn());
   }
 
 public:
-  DEFINE_MDNODE_GET(DILexicalBlock,
-                    (DILocalScope * Scope, DIFile *File, unsigned Line,
-                     unsigned Column),
+  DEFINE_ALL_MDNODE_GET_METHODS(DILexicalBlock,
+                                (DILocalScope * Scope, DIFile *File,
+                                 unsigned Line, unsigned Column),
                                 (Scope, File, Line, Column))
-  DEFINE_MDNODE_GET(DILexicalBlock,
-                    (Metadata * Scope, Metadata *File, unsigned Line,
-                     unsigned Column),
+  DEFINE_ALL_MDNODE_GET_METHODS(DILexicalBlock,
+                                (Metadata * Scope, Metadata *File,
+                                 unsigned Line, unsigned Column),
                                 (Scope, File, Line, Column))
 
   TempDILexicalBlock clone() const { return cloneImpl(); }
 
   unsigned getLine() const { return Line; }
   unsigned getColumn() const { return Column; }
 
   static bool classof(const Metadata *MD) {
     return MD->getMetadataID() == DILexicalBlockKind;
static DILexicalBlockFile *getImpl(LLVMContext &Context, Metadata *Scope,
                                      bool ShouldCreate = true);
 
   TempDILexicalBlockFile cloneImpl() const {
     return getTemporary(getContext(), getScope(), getFile(),
                         getDiscriminator());
   }
 
 public:
-  DEFINE_MDNODE_GET(DILexicalBlockFile,
+  DEFINE_ALL_MDNODE_GET_METHODS(DILexicalBlockFile,
                                 (DILocalScope * Scope, DIFile *File,
                                  unsigned Discriminator),
                                 (Scope, File, Discriminator))
-  DEFINE_MDNODE_GET(DILexicalBlockFile,
-                    (Metadata * Scope, Metadata *File, unsigned Discriminator),
+  DEFINE_ALL_MDNODE_GET_METHODS(DILexicalBlockFile,
+                                (Metadata * Scope, Metadata *File,
+                                 unsigned Discriminator),
                                 (Scope, File, Discriminator))
 
   TempDILexicalBlockFile clone() const { return cloneImpl(); }
   unsigned getDiscriminator() const { return Discriminator; }
 
   static bool classof(const Metadata *MD) {
     return MD->getMetadataID() == DILexicalBlockFileKind;
   }
 };
static DINamespace *getImpl(LLVMContext &Context, Metadata *Scope,
                               StorageType Storage, bool ShouldCreate = true);
 
   TempDINamespace cloneImpl() const {
     return getTemporary(getContext(), getScope(), getName(),
                         getExportSymbols());
   }
 
 public:
-  DEFINE_MDNODE_GET(DINamespace,
-                    (DIScope * Scope, StringRef Name, bool ExportSymbols),
+  DEFINE_ALL_MDNODE_GET_METHODS(DINamespace,
+                                (DIScope * Scope, StringRef Name,
+                                 bool ExportSymbols),
                                 (Scope, Name, ExportSymbols))
-  DEFINE_MDNODE_GET(DINamespace,
-                    (Metadata * Scope, MDString *Name, bool ExportSymbols),
+  DEFINE_ALL_MDNODE_GET_METHODS(DINamespace,
+                                (Metadata * Scope, MDString *Name,
+                                 bool ExportSymbols),
                                 (Scope, Name, ExportSymbols))
 
   TempDINamespace clone() const { return cloneImpl(); }
 
   bool getExportSymbols() const { return ExportSymbols; }
   DIScope *getScope() const { return cast_or_null<DIScope>(getRawScope()); }
   StringRef getName() const { return getStringOperand(2); }
 
   Metadata *getRawScope() const { return getOperand(1); }
class DIModule : public DIScope {
 
   TempDIModule cloneImpl() const {
     return getTemporary(getContext(), getFile(), getScope(), getName(),
                         getConfigurationMacros(), getIncludePath(),
                         getAPINotesFile(), getLineNo(), getIsDecl());
   }
 
 public:
-  DEFINE_MDNODE_GET(DIModule,
+  DEFINE_ALL_MDNODE_GET_METHODS(DIModule,
                                 (DIFile * File, DIScope *Scope, StringRef Name,
-                     StringRef ConfigurationMacros, StringRef IncludePath,
-                     StringRef APINotesFile, unsigned LineNo,
-                     bool IsDecl = false),
-                    (File, Scope, Name, ConfigurationMacros, IncludePath,
-                     APINotesFile, LineNo, IsDecl))
-  DEFINE_MDNODE_GET(DIModule,
-                    (Metadata * File, Metadata *Scope, MDString *Name,
-                     MDString *ConfigurationMacros, MDString *IncludePath,
-                     MDString *APINotesFile, unsigned LineNo,
-                     bool IsDecl = false),
-                    (File, Scope, Name, ConfigurationMacros, IncludePath,
-                     APINotesFile, LineNo, IsDecl))
+                                 StringRef ConfigurationMacros,
+                                 StringRef IncludePath, StringRef APINotesFile,
+                                 unsigned LineNo, bool IsDecl = false),
+                                (File, Scope, Name, ConfigurationMacros,
+                                 IncludePath, APINotesFile, LineNo, IsDecl))
+  DEFINE_ALL_MDNODE_GET_METHODS(DIModule,
+                                (Metadata * File, Metadata *Scope,
+                                 MDString *Name, MDString *ConfigurationMacros,
+                                 MDString *IncludePath, MDString *APINotesFile,
+                                 unsigned LineNo, bool IsDecl = false),
+                                (File, Scope, Name, ConfigurationMacros,
+                                 IncludePath, APINotesFile, LineNo, IsDecl))
 
   TempDIModule clone() const { return cloneImpl(); }
 
   DIScope *getScope() const { return cast_or_null<DIScope>(getRawScope()); }
   StringRef getName() const { return getStringOperand(2); }
   StringRef getConfigurationMacros() const { return getStringOperand(3); }
   StringRef getIncludePath() const { return getStringOperand(4); }
   StringRef getAPINotesFile() const { return getStringOperand(5); }
static DITemplateTypeParameter *getImpl(LLVMContext &Context, MDString *Name,
                                           StorageType Storage,
                                           bool ShouldCreate = true);
 
   TempDITemplateTypeParameter cloneImpl() const {
     return getTemporary(getContext(), getName(), getType(), isDefault());
   }
 
 public:
-  DEFINE_MDNODE_GET(DITemplateTypeParameter,
+  DEFINE_ALL_MDNODE_GET_METHODS(DITemplateTypeParameter,
                                 (StringRef Name, DIType *Type, bool IsDefault),
                                 (Name, Type, IsDefault))
-  DEFINE_MDNODE_GET(DITemplateTypeParameter,
-                    (MDString * Name, Metadata *Type, bool IsDefault),
+  DEFINE_ALL_MDNODE_GET_METHODS(DITemplateTypeParameter,
+                                (MDString * Name, Metadata *Type,
+                                 bool IsDefault),
                                 (Name, Type, IsDefault))
 
   TempDITemplateTypeParameter clone() const { return cloneImpl(); }
 
   static bool classof(const Metadata *MD) {
     return MD->getMetadataID() == DITemplateTypeParameterKind;
   }
 };
 
static DITemplateValueParameter *getImpl(LLVMContext &Context, unsigned Tag,
                                            bool ShouldCreate = true);
 
   TempDITemplateValueParameter cloneImpl() const {
     return getTemporary(getContext(), getTag(), getName(), getType(),
                         isDefault(), getValue());
   }
 
 public:
-  DEFINE_MDNODE_GET(DITemplateValueParameter,
-                    (unsigned Tag, StringRef Name, DIType *Type, bool IsDefault,
-                     Metadata *Value),
+  DEFINE_ALL_MDNODE_GET_METHODS(DITemplateValueParameter,
+                                (unsigned Tag, StringRef Name, DIType *Type,
+                                 bool IsDefault, Metadata *Value),
                                 (Tag, Name, Type, IsDefault, Value))
-  DEFINE_MDNODE_GET(DITemplateValueParameter,
+  DEFINE_ALL_MDNODE_GET_METHODS(DITemplateValueParameter,
                                 (unsigned Tag, MDString *Name, Metadata *Type,
                                  bool IsDefault, Metadata *Value),
                                 (Tag, Name, Type, IsDefault, Value))
 
   TempDITemplateValueParameter clone() const { return cloneImpl(); }
 
   Metadata *getValue() const { return getOperand(2); }
 
   static bool classof(const Metadata *MD) {
     return MD->getMetadataID() == DITemplateValueParameterKind;
   }
static DIExpression *getImpl(LLVMContext &Context,
                                ArrayRef<uint64_t> Elements, StorageType Storage,
                                bool ShouldCreate = true);
 
   TempDIExpression cloneImpl() const {
     return getTemporary(getContext(), getElements());
   }
 
 public:
-  DEFINE_MDNODE_GET(DIExpression, (ArrayRef<uint64_t> Elements), (Elements))
+  DEFINE_ALWAYS_UNIQUED_MDNODE_GET_METHODS(DIExpression,
+                                           (ArrayRef<uint64_t> Elements),
+                                           (Elements))
 
   TempDIExpression clone() const { return cloneImpl(); }
 
   ArrayRef<uint64_t> getElements() const { return Elements; }
 
   unsigned getNumElements() const { return Elements.size(); }
 
   uint64_t getElement(unsigned I) const {
public:
   /// is fragmented across multiple Values. The DW_OP_LLVM_fragment operation
   /// will be appended to the elements of \c Expr. If \c Expr already contains
   /// a \c DW_OP_LLVM_fragment \c OffsetInBits is interpreted as an offset
   /// into the existing fragment.
   ///
   /// \param OffsetInBits Offset of the piece in bits.
   /// \param SizeInBits   Size of the piece in bits.
   /// \return             Creating a fragment expression may fail if \c Expr
-  ///                     contains arithmetic operations that would be
-  ///                     truncated.
+  ///                     contains arithmetic operations that would be truncated.
   static Optional<DIExpression *>
   createFragmentExpression(const DIExpression *Expr, unsigned OffsetInBits,
                            unsigned SizeInBits);
 
   /// Determine the relative position of the fragments passed in.
   /// Returns -1 if this is entirely before Other, 0 if this and Other overlap,
   /// 1 if this is entirely after Other.
   static int fragmentCmp(const FragmentInfo &A, const FragmentInfo &B) {
TempDIGlobalVariable cloneImpl() const {
     return getTemporary(getContext(), getScope(), getName(), getLinkageName(),
                         getFile(), getLine(), getType(), isLocalToUnit(),
                         isDefinition(), getStaticDataMemberDeclaration(),
                         getTemplateParams(), getAlignInBits(),
                         getAnnotations());
   }
 
 public:
-  DEFINE_MDNODE_GET(
+  DEFINE_ALL_MDNODE_GET_METHODS(
       DIGlobalVariable,
       (DIScope * Scope, StringRef Name, StringRef LinkageName, DIFile *File,
        unsigned Line, DIType *Type, bool IsLocalToUnit, bool IsDefinition,
        DIDerivedType *StaticDataMemberDeclaration, MDTuple *TemplateParams,
        uint32_t AlignInBits, DINodeArray Annotations),
       (Scope, Name, LinkageName, File, Line, Type, IsLocalToUnit, IsDefinition,
        StaticDataMemberDeclaration, TemplateParams, AlignInBits, Annotations))
-  DEFINE_MDNODE_GET(
+  DEFINE_ALL_MDNODE_GET_METHODS(
       DIGlobalVariable,
       (Metadata * Scope, MDString *Name, MDString *LinkageName, Metadata *File,
        unsigned Line, Metadata *Type, bool IsLocalToUnit, bool IsDefinition,
        Metadata *StaticDataMemberDeclaration, Metadata *TemplateParams,
        uint32_t AlignInBits, Metadata *Annotations),
       (Scope, Name, LinkageName, File, Line, Type, IsLocalToUnit, IsDefinition,
        StaticDataMemberDeclaration, TemplateParams, AlignInBits, Annotations))
 
class DICommonBlock : public DIScope {
   unsigned LineNo;
 
   friend class LLVMContextImpl;
   friend class MDNode;
 
   DICommonBlock(LLVMContext &Context, StorageType Storage, unsigned LineNo,
                 ArrayRef<Metadata *> Ops)
       : DIScope(Context, DICommonBlockKind, Storage, dwarf::DW_TAG_common_block,
-                Ops),
-        LineNo(LineNo) {}
+                Ops), LineNo(LineNo) {}
 
   static DICommonBlock *getImpl(LLVMContext &Context, DIScope *Scope,
                                 DIGlobalVariable *Decl, StringRef Name,
                                 DIFile *File, unsigned LineNo,
-                                StorageType Storage, bool ShouldCreate = true) {
+                                StorageType Storage,
+                                bool ShouldCreate = true) {
     return getImpl(Context, Scope, Decl, getCanonicalMDString(Context, Name),
                    File, LineNo, Storage, ShouldCreate);
   }
   static DICommonBlock *getImpl(LLVMContext &Context, Metadata *Scope,
                                 Metadata *Decl, MDString *Name, Metadata *File,
-                                unsigned LineNo, StorageType Storage,
-                                bool ShouldCreate = true);
+                                unsigned LineNo, 
+                                StorageType Storage, bool ShouldCreate = true);
 
   TempDICommonBlock cloneImpl() const {
     return getTemporary(getContext(), getScope(), getDecl(), getName(),
                         getFile(), getLineNo());
   }
 
 public:
-  DEFINE_MDNODE_GET(DICommonBlock,
-                    (DIScope * Scope, DIGlobalVariable *Decl, StringRef Name,
-                     DIFile *File, unsigned LineNo),
+  DEFINE_ALL_MDNODE_GET_METHODS(DICommonBlock,
+                                (DIScope * Scope, DIGlobalVariable *Decl,
+                                 StringRef Name, DIFile *File, unsigned LineNo),
                                 (Scope, Decl, Name, File, LineNo))
-  DEFINE_MDNODE_GET(DICommonBlock,
-                    (Metadata * Scope, Metadata *Decl, MDString *Name,
-                     Metadata *File, unsigned LineNo),
+  DEFINE_ALL_MDNODE_GET_METHODS(DICommonBlock,
+                                (Metadata * Scope, Metadata *Decl,
+                                 MDString *Name, Metadata *File,
+                                 unsigned LineNo),
                                 (Scope, Decl, Name, File, LineNo))
 
   TempDICommonBlock clone() const { return cloneImpl(); }
 
   DIScope *getScope() const { return cast_or_null<DIScope>(getRawScope()); }
   DIGlobalVariable *getDecl() const {
     return cast_or_null<DIGlobalVariable>(getRawDecl());
   }
   StringRef getName() const { return getStringOperand(2); }
DILocalVariable(LLVMContext &C, StorageType Storage, unsigned Line,
     assert(Arg < (1 << 16) && "DILocalVariable: Arg out of range");
   }
   ~DILocalVariable() = default;
 
   static DILocalVariable *getImpl(LLVMContext &Context, DIScope *Scope,
                                   StringRef Name, DIFile *File, unsigned Line,
                                   DIType *Type, unsigned Arg, DIFlags Flags,
                                   uint32_t AlignInBits, DINodeArray Annotations,
-                                  StorageType Storage,
-                                  bool ShouldCreate = true) {
+                                  StorageType Storage, bool ShouldCreate = true) {
     return getImpl(Context, Scope, getCanonicalMDString(Context, Name), File,
-                   Line, Type, Arg, Flags, AlignInBits, Annotations.get(),
-                   Storage, ShouldCreate);
+                   Line, Type, Arg, Flags, AlignInBits, Annotations.get(), Storage,
+                   ShouldCreate);
   }
   static DILocalVariable *getImpl(LLVMContext &Context, Metadata *Scope,
                                   MDString *Name, Metadata *File, unsigned Line,
                                   Metadata *Type, unsigned Arg, DIFlags Flags,
                                   uint32_t AlignInBits, Metadata *Annotations,
-                                  StorageType Storage,
-                                  bool ShouldCreate = true);
+                                  StorageType Storage, bool ShouldCreate = true);
 
   TempDILocalVariable cloneImpl() const {
     return getTemporary(getContext(), getScope(), getName(), getFile(),
                         getLine(), getType(), getArg(), getFlags(),
                         getAlignInBits(), getAnnotations());
   }
 
 public:
-  DEFINE_MDNODE_GET(DILocalVariable,
-                    (DILocalScope * Scope, StringRef Name, DIFile *File,
-                     unsigned Line, DIType *Type, unsigned Arg, DIFlags Flags,
+  DEFINE_ALL_MDNODE_GET_METHODS(DILocalVariable,
+                                (DILocalScope * Scope, StringRef Name,
+                                 DIFile *File, unsigned Line, DIType *Type,
+                                 unsigned Arg, DIFlags Flags,
                                  uint32_t AlignInBits, DINodeArray Annotations),
-                    (Scope, Name, File, Line, Type, Arg, Flags, AlignInBits,
-                     Annotations))
-  DEFINE_MDNODE_GET(DILocalVariable,
-                    (Metadata * Scope, MDString *Name, Metadata *File,
-                     unsigned Line, Metadata *Type, unsigned Arg, DIFlags Flags,
+                                (Scope, Name, File, Line, Type, Arg, Flags,
+                                 AlignInBits, Annotations))
+  DEFINE_ALL_MDNODE_GET_METHODS(DILocalVariable,
+                                (Metadata * Scope, MDString *Name,
+                                 Metadata *File, unsigned Line, Metadata *Type,
+                                 unsigned Arg, DIFlags Flags,
                                  uint32_t AlignInBits, Metadata *Annotations),
-                    (Scope, Name, File, Line, Type, Arg, Flags, AlignInBits,
-                     Annotations))
+                                (Scope, Name, File, Line, Type, Arg, Flags,
+                                 AlignInBits, Annotations))
 
   TempDILocalVariable clone() const { return cloneImpl(); }
 
   /// Get the local scope for this variable.
   ///
   /// Variables must be defined in a local scope.
   DILocalScope *getScope() const {
     return cast<DILocalScope>(DIVariable::getScope());
class DILabel : public DINode {
 
   unsigned Line;
 
   DILabel(LLVMContext &C, StorageType Storage, unsigned Line,
           ArrayRef<Metadata *> Ops)
       : DINode(C, DILabelKind, Storage, dwarf::DW_TAG_label, Ops), Line(Line) {}
   ~DILabel() = default;
 
-  static DILabel *getImpl(LLVMContext &Context, DIScope *Scope, StringRef Name,
-                          DIFile *File, unsigned Line, StorageType Storage,
+  static DILabel *getImpl(LLVMContext &Context, DIScope *Scope,
+                          StringRef Name, DIFile *File, unsigned Line,
+                          StorageType Storage,
                           bool ShouldCreate = true) {
     return getImpl(Context, Scope, getCanonicalMDString(Context, Name), File,
                    Line, Storage, ShouldCreate);
   }
-  static DILabel *getImpl(LLVMContext &Context, Metadata *Scope, MDString *Name,
-                          Metadata *File, unsigned Line, StorageType Storage,
+  static DILabel *getImpl(LLVMContext &Context, Metadata *Scope,
+                          MDString *Name, Metadata *File, unsigned Line,
+                          StorageType Storage,
                           bool ShouldCreate = true);
 
   TempDILabel cloneImpl() const {
     return getTemporary(getContext(), getScope(), getName(), getFile(),
                         getLine());
   }
 
 public:
-  DEFINE_MDNODE_GET(DILabel,
-                    (DILocalScope * Scope, StringRef Name, DIFile *File,
-                     unsigned Line),
+  DEFINE_ALL_MDNODE_GET_METHODS(DILabel,
+                                (DILocalScope * Scope, StringRef Name,
+                                 DIFile *File, unsigned Line),
                                 (Scope, Name, File, Line))
-  DEFINE_MDNODE_GET(DILabel,
-                    (Metadata * Scope, MDString *Name, Metadata *File,
-                     unsigned Line),
+  DEFINE_ALL_MDNODE_GET_METHODS(DILabel,
+                                (Metadata * Scope, MDString *Name,
+                                 Metadata *File, unsigned Line),
                                 (Scope, Name, File, Line))
 
   TempDILabel clone() const { return cloneImpl(); }
 
   /// Get the local scope for this label.
   ///
   /// Labels must be defined in a local scope.
   DILocalScope *getScope() const {
     return cast_or_null<DILocalScope>(getRawScope());
class DIObjCProperty : public DINode {
 
   TempDIObjCProperty cloneImpl() const {
     return getTemporary(getContext(), getName(), getFile(), getLine(),
                         getGetterName(), getSetterName(), getAttributes(),
                         getType());
   }
 
 public:
-  DEFINE_MDNODE_GET(DIObjCProperty,
+  DEFINE_ALL_MDNODE_GET_METHODS(DIObjCProperty,
                                 (StringRef Name, DIFile *File, unsigned Line,
                                  StringRef GetterName, StringRef SetterName,
                                  unsigned Attributes, DIType *Type),
-                    (Name, File, Line, GetterName, SetterName, Attributes,
-                     Type))
-  DEFINE_MDNODE_GET(DIObjCProperty,
+                                (Name, File, Line, GetterName, SetterName,
+                                 Attributes, Type))
+  DEFINE_ALL_MDNODE_GET_METHODS(DIObjCProperty,
                                 (MDString * Name, Metadata *File, unsigned Line,
                                  MDString *GetterName, MDString *SetterName,
                                  unsigned Attributes, Metadata *Type),
-                    (Name, File, Line, GetterName, SetterName, Attributes,
-                     Type))
+                                (Name, File, Line, GetterName, SetterName,
+                                 Attributes, Type))
 
   TempDIObjCProperty clone() const { return cloneImpl(); }
 
   unsigned getLine() const { return Line; }
   unsigned getAttributes() const { return Attributes; }
   StringRef getName() const { return getStringOperand(0); }
   DIFile *getFile() const { return cast_or_null<DIFile>(getRawFile()); }
   StringRef getGetterName() const { return getStringOperand(2); }
getImpl(LLVMContext &Context, unsigned Tag, Metadata *Scope, Metadata *Entity,
           StorageType Storage, bool ShouldCreate = true);
 
   TempDIImportedEntity cloneImpl() const {
     return getTemporary(getContext(), getTag(), getScope(), getEntity(),
                         getFile(), getLine(), getName(), getElements());
   }
 
 public:
-  DEFINE_MDNODE_GET(DIImportedEntity,
-                    (unsigned Tag, DIScope *Scope, DINode *Entity, DIFile *File,
-                     unsigned Line, StringRef Name = "",
-                     DINodeArray Elements = nullptr),
-                    (Tag, Scope, Entity, File, Line, Name, Elements))
-  DEFINE_MDNODE_GET(DIImportedEntity,
-                    (unsigned Tag, Metadata *Scope, Metadata *Entity,
-                     Metadata *File, unsigned Line, MDString *Name,
-                     Metadata *Elements = nullptr),
-                    (Tag, Scope, Entity, File, Line, Name, Elements))
+  DEFINE_ALL_MDNODE_GET_METHODS(
+      DIImportedEntity,
+       unsigned Line, StringRef Name = "", DINodeArray Elements = nullptr),
+  DEFINE_ALL_MDNODE_GET_METHODS(
+      DIImportedEntity,
+      (unsigned Tag, Metadata *Scope, Metadata *Entity, Metadata *File,
+       unsigned Line, MDString *Name, Metadata *Elements = nullptr),
 
   TempDIImportedEntity clone() const { return cloneImpl(); }
 
   unsigned getLine() const { return Line; }
   DIScope *getScope() const { return cast_or_null<DIScope>(getRawScope()); }
   DINode *getEntity() const { return cast_or_null<DINode>(getRawEntity()); }
   StringRef getName() const { return getStringOperand(2); }
   DIFile *getFile() const { return cast_or_null<DIFile>(getRawFile()); }
class DIGlobalVariableExpression : public MDNode {
   getImpl(LLVMContext &Context, Metadata *Variable, Metadata *Expression,
           StorageType Storage, bool ShouldCreate = true);
 
   TempDIGlobalVariableExpression cloneImpl() const {
     return getTemporary(getContext(), getVariable(), getExpression());
   }
 
 public:
-  DEFINE_MDNODE_GET(DIGlobalVariableExpression,
+  DEFINE_ALL_MDNODE_GET_METHODS(DIGlobalVariableExpression,
                                 (Metadata * Variable, Metadata *Expression),
                                 (Variable, Expression))
 
   TempDIGlobalVariableExpression clone() const { return cloneImpl(); }
 
   Metadata *getRawVariable() const { return getOperand(0); }
 
   DIGlobalVariable *getVariable() const {
     return cast_or_null<DIGlobalVariable>(getRawVariable());
   }
static DIMacro *getImpl(LLVMContext &Context, unsigned MIType, unsigned Line,
                           bool ShouldCreate = true);
 
   TempDIMacro cloneImpl() const {
     return getTemporary(getContext(), getMacinfoType(), getLine(), getName(),
                         getValue());
   }
 
 public:
-  DEFINE_MDNODE_GET(DIMacro,
+  DEFINE_ALL_MDNODE_GET_METHODS(DIMacro,
                                 (unsigned MIType, unsigned Line, StringRef Name,
                                  StringRef Value = ""),
                                 (MIType, Line, Name, Value))
-  DEFINE_MDNODE_GET(DIMacro,
+  DEFINE_ALL_MDNODE_GET_METHODS(DIMacro,
                                 (unsigned MIType, unsigned Line, MDString *Name,
                                  MDString *Value),
                                 (MIType, Line, Name, Value))
 
   TempDIMacro clone() const { return cloneImpl(); }
 
   unsigned getLine() const { return Line; }
 
   StringRef getName() const { return getStringOperand(0); }
   StringRef getValue() const { return getStringOperand(1); }
 
static DIMacroFile *getImpl(LLVMContext &Context, unsigned MIType,
                               StorageType Storage, bool ShouldCreate = true);
 
   TempDIMacroFile cloneImpl() const {
     return getTemporary(getContext(), getMacinfoType(), getLine(), getFile(),
                         getElements());
   }
 
 public:
-  DEFINE_MDNODE_GET(DIMacroFile,
+  DEFINE_ALL_MDNODE_GET_METHODS(DIMacroFile,
                                 (unsigned MIType, unsigned Line, DIFile *File,
                                  DIMacroNodeArray Elements),
                                 (MIType, Line, File, Elements))
-  DEFINE_MDNODE_GET(DIMacroFile,
+  DEFINE_ALL_MDNODE_GET_METHODS(DIMacroFile,
                                 (unsigned MIType, unsigned Line, Metadata *File,
                                  Metadata *Elements),
                                 (MIType, Line, File, Elements))
 
   TempDIMacroFile clone() const { return cloneImpl(); }
 
   void replaceElements(DIMacroNodeArray Elements) {
 #ifndef NDEBUG
     for (DIMacroNode *Op : getElements())
       assert(is_contained(Elements->operands(), Op) &&
              "Lost a macro node during macro node list replacement");
#endif
 
   static bool classof(const Metadata *MD) {
     return MD->getMetadataID() == DIMacroFileKind;
   }
 };
 
 /// List of ValueAsMetadata, to be used as an argument to a dbg.value
 /// intrinsic.
-class DIArgList : public MDNode {
+class DIArgList : public Metadata, public ReplaceableMetadataImpl {
   friend class LLVMContextImpl;
   friend class MDNode;
   using iterator = SmallVectorImpl<ValueAsMetadata *>::iterator;
 
   SmallVector<ValueAsMetadata *, 4> Args;
 
-  DIArgList(LLVMContext &C, StorageType Storage,
-            ArrayRef<ValueAsMetadata *> Args)
-      : MDNode(C, DIArgListKind, Storage, None),
+  DIArgList(LLVMContext &C, ArrayRef<ValueAsMetadata *> Args)
+      : Metadata(DIArgListKind, Uniqued), ReplaceableMetadataImpl(C),
         Args(Args.begin(), Args.end()) {
     track();
   }
-  ~DIArgList() { untrack(); }
+  ~DIArgList() {
+    untrack();
+    ReplaceableMetadataImpl::resolveAllUses(/* ResolveUsers */ false);
+  }
 
   static DIArgList *getImpl(LLVMContext &Context,
                             ArrayRef<ValueAsMetadata *> Args,
-                            StorageType Storage, bool ShouldCreate = true);
-
-  TempDIArgList cloneImpl() const {
-    return getTemporary(getContext(), getArgs());
-  }
+                            bool ShouldCreate = true);
 
   void track();
   void untrack();
   void dropAllReferences();
 
 public:
-  DEFINE_MDNODE_GET(DIArgList, (ArrayRef<ValueAsMetadata *> Args), (Args))
-
-  TempDIArgList clone() const { return cloneImpl(); }
+  static DIArgList *get(LLVMContext &Context,
+                        ArrayRef<ValueAsMetadata *> Args) {
+    return getImpl(Context, Args);
+  }
+  static DIArgList *getIfExists(LLVMContext &Context,
+                                ArrayRef<ValueAsMetadata *> Args) {
+    return getImpl(Context, Args, false);
+  }
 
   ArrayRef<ValueAsMetadata *> getArgs() const { return Args; }
 
   iterator args_begin() { return Args.begin(); }
   iterator args_end() { return Args.end(); }
 
   static bool classof(const Metadata *MD) {
     return MD->getMetadataID() == DIArgListKind;
template <> struct DenseMapInfo<DebugVariable> {
   }
 };
 
 } // end namespace llvm
 
 #undef DEFINE_MDNODE_GET_UNPACK_IMPL
 #undef DEFINE_MDNODE_GET_UNPACK
 #undef DEFINE_MDNODE_GET
+#undef DEFINE_MDNODE_GET_IF_EXISTS
+#undef DEFINE_MDNODE_GET_DISTINCT
+#undef DEFINE_MDNODE_GET_TEMPORARY
+#undef DEFINE_ALL_MDNODE_GET_METHODS
+#undef DEFINE_ALWAYS_DISTINCT_MDNODE_GET_METHODS
+#undef DEFINE_ALWAYS_UNIQUED_MDNODE_GET_METHODS
 
 #endif // LLVM_IR_DEBUGINFOMETADATA_H

llvm/include/llvm/IR/Metadata.h

 /// A member of the \a Value hierarchy to represent a reference to metadata.
 /// This allows, e.g., instrinsics to have metadata as operands.
 ///
 /// Notably, this is the only thing in either hierarchy that is allowed to
 /// reference \a LocalAsMetadata.
 class MetadataAsValue : public Value {
   friend class ReplaceableMetadataImpl;
   friend class LLVMContextImpl;
+  friend class DIArgList;
 
   Metadata *MD;
 
   MetadataAsValue(Type *Ty, Metadata *MD);
 
   /// Drop use of metadata (during teardown).
   void dropUse() { MD = nullptr; }
 

llvm/include/llvm/IR/Metadata.def

 //===- llvm/IR/Metadata.def - Metadata definitions --------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Macros for running through all types of metadata.
 //
+// Definitions for terms used to describe metadata include:
+//
+// * BRANCH: refers to an "abstract" metadata kind, which exists only in the
+//   C++ class hierarchy. These cannot appear directly in IR/bitcode.
+// * LEAF: refers to a "concrete" metadata kind. These can appear directly in
+//   IR/bitcode.
+// * SPECIALIZED: refers to non-MDTuple MDNodes, i.e. those that use the
+//   syntax "!CLASS(...)" in IR.
+// * UNIQUABLE: refers to nodes which can use uniqued, distinct, or temporary
+//   storage without any restrictions.
+// * UNIQUED: refers to nodes which must use uniqued or temporary storage.
+// * DISTINCT: refers to nodes which must use distinct or temporary storage.
+//
+// In LLVM IR, UNIQUABLE and DISTINCT nodes must be referred to by MDNode ID,
+// as in `!0`, whereas UNIQUED nodes canonically appear inline at each use, as
+// in `DIExpression(...)`. This is because `distinct` nodes maintain their
+// identity irrespective of contents, making the inline syntax ambiguous in
+// some cases.
+//
+// Note: UNIQUABLE, UNIQUED, and DISTINCT are mutually exclusive. For example,
+// code which intends to consider all nodes which can use uniqued storage must
+// consider both UNIQUABLE and UNIQUED nodes.
+//
 //===----------------------------------------------------------------------===//
 
 #if !(defined HANDLE_METADATA || defined HANDLE_METADATA_LEAF ||               \
       defined HANDLE_METADATA_BRANCH || defined HANDLE_MDNODE_LEAF ||          \
-      defined HANDLE_MDNODE_LEAF_UNIQUABLE || defined HANDLE_MDNODE_BRANCH ||  \
+      defined HANDLE_MDNODE_LEAF_UNIQUABLE ||                                  \
+      defined HANDLE_MDNODE_LEAF_UNIQUED ||                                    \
+      defined HANDLE_MDNODE_LEAF_DISTINCT || defined HANDLE_MDNODE_BRANCH ||   \
       defined HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE ||                      \
+      defined HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUED ||                        \
+      defined HANDLE_SPECIALIZED_MDNODE_LEAF_DISTINCT ||                       \
       defined HANDLE_SPECIALIZED_MDNODE_LEAF ||                                \
       defined HANDLE_SPECIALIZED_MDNODE_BRANCH)
 #error "Missing macro definition of HANDLE_METADATA*"
 #endif
 
 // Handler for all types of metadata.
 #ifndef HANDLE_METADATA
 #define HANDLE_METADATA(CLASS)
 #endif
 
 // Handler for leaf nodes in the class hierarchy.
 #ifndef HANDLE_METADATA_LEAF
 #define HANDLE_METADATA_LEAF(CLASS) HANDLE_METADATA(CLASS)
 #endif
 
 // Handler for non-leaf nodes in the class hierarchy.
 #ifndef HANDLE_METADATA_BRANCH
 #define HANDLE_METADATA_BRANCH(CLASS) HANDLE_METADATA(CLASS)
 #endif
 
-// Handler for specialized and uniquable leaf nodes under MDNode.  Defers to
+// Handler for specialized and uniquable leaf nodes under MDNode. Defers to
 // HANDLE_MDNODE_LEAF_UNIQUABLE if it's defined, otherwise to
 // HANDLE_SPECIALIZED_MDNODE_LEAF.
 #ifndef HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE
 #ifdef HANDLE_MDNODE_LEAF_UNIQUABLE
 #define HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(CLASS)                        \
   HANDLE_MDNODE_LEAF_UNIQUABLE(CLASS)
 #else
 #define HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(CLASS)                        \
   HANDLE_SPECIALIZED_MDNODE_LEAF(CLASS)
 #endif
 #endif
 
-// Handler for leaf nodes under MDNode.
+// Handler for specialized and always-uniqued leaf nodes under MDNode. Defers to
+// HANDLE_MDNODE_LEAF_UNIQUED if it's defined, otherwise to
+// HANDLE_SPECIALIZED_MDNODE_LEAF.
+#ifndef HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUED
+#ifdef HANDLE_MDNODE_LEAF_UNIQUED
+#define HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUED(CLASS)                        \
+  HANDLE_MDNODE_LEAF_UNIQUED(CLASS)
+#else
+#define HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUED(CLASS)                        \
+  HANDLE_SPECIALIZED_MDNODE_LEAF(CLASS)
+#endif
+#endif
+
+// Handler for specialized and always-distinct leaf nodes under MDNode. Defers
+// to HANDLE_MDNODE_LEAF_DISTINCT if it's defined, otherwise to
+// HANDLE_SPECIALIZED_MDNODE_LEAF.
+#ifndef HANDLE_SPECIALIZED_MDNODE_LEAF_DISTINCT
+#ifdef HANDLE_MDNODE_LEAF_DISTINCT
+#define HANDLE_SPECIALIZED_MDNODE_LEAF_DISTINCT(CLASS)                        \
+  HANDLE_MDNODE_LEAF_DISTINCT(CLASS)
+#else
+#define HANDLE_SPECIALIZED_MDNODE_LEAF_DISTINCT(CLASS)                        \
+  HANDLE_SPECIALIZED_MDNODE_LEAF(CLASS)
+#endif
+#endif
+
+// Handler for uniquable leaf nodes under MDNode.
 #ifndef HANDLE_MDNODE_LEAF_UNIQUABLE
 #define HANDLE_MDNODE_LEAF_UNIQUABLE(CLASS) HANDLE_MDNODE_LEAF(CLASS)
 #endif
 
+// Handler for uniqued leaf nodes under MDNode.
+#ifndef HANDLE_MDNODE_LEAF_UNIQUED
+#define HANDLE_MDNODE_LEAF_UNIQUED(CLASS) HANDLE_MDNODE_LEAF(CLASS)
+#endif
+
+// Handler for distinct leaf nodes under MDNode.
+#ifndef HANDLE_MDNODE_LEAF_DISTINCT
+#define HANDLE_MDNODE_LEAF_DISTINCT(CLASS) HANDLE_MDNODE_LEAF(CLASS)
+#endif
+
 // Handler for leaf nodes under MDNode.
 #ifndef HANDLE_MDNODE_LEAF
 #define HANDLE_MDNODE_LEAF(CLASS) HANDLE_METADATA_LEAF(CLASS)
 #endif
 
 // Handler for non-leaf nodes under MDNode.
 #ifndef HANDLE_MDNODE_BRANCH
 #define HANDLE_MDNODE_BRANCH(CLASS) HANDLE_METADATA_BRANCH(CLASS)

 #define HANDLE_SPECIALIZED_MDNODE_BRANCH(CLASS) HANDLE_MDNODE_BRANCH(CLASS)
 #endif
 
 HANDLE_METADATA_LEAF(MDString)
 HANDLE_METADATA_BRANCH(ValueAsMetadata)
 HANDLE_METADATA_LEAF(ConstantAsMetadata)
 HANDLE_METADATA_LEAF(LocalAsMetadata)
 HANDLE_METADATA_LEAF(DistinctMDOperandPlaceholder)
+HANDLE_METADATA_LEAF(DIArgList)
 HANDLE_MDNODE_BRANCH(MDNode)
 HANDLE_MDNODE_LEAF_UNIQUABLE(MDTuple)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DILocation)
-HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DIExpression)
+HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUED(DIExpression)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DIGlobalVariableExpression)
 HANDLE_SPECIALIZED_MDNODE_BRANCH(DINode)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(GenericDINode)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DISubrange)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DIEnumerator)
 HANDLE_SPECIALIZED_MDNODE_BRANCH(DIScope)
 HANDLE_SPECIALIZED_MDNODE_BRANCH(DIType)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DIBasicType)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DIDerivedType)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DICompositeType)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DISubroutineType)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DIFile)
-HANDLE_SPECIALIZED_MDNODE_LEAF(DICompileUnit)
+HANDLE_SPECIALIZED_MDNODE_LEAF_DISTINCT(DICompileUnit)
 HANDLE_SPECIALIZED_MDNODE_BRANCH(DILocalScope)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DISubprogram)
 HANDLE_SPECIALIZED_MDNODE_BRANCH(DILexicalBlockBase)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DILexicalBlock)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DILexicalBlockFile)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DINamespace)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DIModule)
 HANDLE_SPECIALIZED_MDNODE_BRANCH(DITemplateParameter)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DITemplateTypeParameter)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DITemplateValueParameter)
 HANDLE_SPECIALIZED_MDNODE_BRANCH(DIVariable)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DIGlobalVariable)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DILocalVariable)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DILabel)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DIObjCProperty)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DIImportedEntity)
 HANDLE_SPECIALIZED_MDNODE_BRANCH(DIMacroNode)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DIMacro)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DIMacroFile)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DICommonBlock)
-HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DIArgList)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DIStringType)
 HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DIGenericSubrange)
 
 #undef HANDLE_METADATA
 #undef HANDLE_METADATA_LEAF
 #undef HANDLE_METADATA_BRANCH
 #undef HANDLE_MDNODE_LEAF
 #undef HANDLE_MDNODE_LEAF_UNIQUABLE
+#undef HANDLE_MDNODE_LEAF_UNIQUED
+#undef HANDLE_MDNODE_LEAF_DISTINCT
 #undef HANDLE_MDNODE_BRANCH
 #undef HANDLE_SPECIALIZED_MDNODE_LEAF
 #undef HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE
+#undef HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUED
+#undef HANDLE_SPECIALIZED_MDNODE_LEAF_DISTINCT
 #undef HANDLE_SPECIALIZED_MDNODE_BRANCH

llvm/lib/AsmParser/LLParser.cpp

bool LLParser::parseNamedMetadata() {
   Lex.Lex();
 
   if (parseToken(lltok::equal, "expected '=' here") ||
       parseToken(lltok::exclaim, "Expected '!' here") ||
       parseToken(lltok::lbrace, "Expected '{' here"))
     return true;
 
   NamedMDNode *NMD = M->getOrInsertNamedMetadata(Name);
-  if (Lex.getKind() != lltok::rbrace)
-    do {
-      MDNode *N = nullptr;
-      // parse DIExpressions inline as a special case. They are still MDNodes,
-      // so they can still appear in named metadata. Remove this logic if they
-      // become plain Metadata.
-      if (Lex.getKind() == lltok::MetadataVar &&
-          Lex.getStrVal() == "DIExpression") {
-        if (parseDIExpression(N, /*IsDistinct=*/false))
-          return true;
-        // DIArgLists should only appear inline in a function, as they may
-        // contain LocalAsMetadata arguments which require a function context.
-      } else if (Lex.getKind() == lltok::MetadataVar &&
-                 Lex.getStrVal() == "DIArgList") {
-        return tokError("found DIArgList outside of function");
-      } else if (parseToken(lltok::exclaim, "Expected '!' here") ||
-                 parseMDNodeID(N)) {
-        return true;
-      }
-      NMD->addOperand(N);
-    } while (EatIfPresent(lltok::comma));
+
+  if (Lex.getKind() == lltok::rbrace) {
+    Lex.Lex();
+    return false;
+  }
+
+    // Parse uniqued MDNodes inline as a special case.
+#define HANDLE_MDNODE_LEAF_UNIQUED(CLASS)                                      \
+  if (Lex.getKind() == lltok::MetadataVar && Lex.getStrVal() == #CLASS) {      \
+    if (parse##CLASS(N, /*IsDistinct=*/false))                                 \
+      return true;                                                             \
+    NMD->addOperand(N);                                                        \
+    continue;                                                                  \
+  }
+#include "llvm/IR/Metadata.def"
+    // Parse all other MDNodes as an MDNodeID.
+    if (parseToken(lltok::exclaim, "Expected '!' here") || parseMDNodeID(N)) {
 
   return parseToken(lltok::rbrace, "expected end of metadata node");
 }
 
 /// parseStandaloneMetadata:
 ///   !42 = !{...}
 bool LLParser::parseStandaloneMetadata() {
   assert(Lex.getKind() == lltok::exclaim);
   Lex.Lex();
   unsigned MetadataID = 0;
 
   MDNode *Init;
   if (parseUInt32(MetadataID) || parseToken(lltok::equal, "expected '=' here"))
     return true;
 
   // Detect common error, from old metadata syntax.
   if (Lex.getKind() == lltok::Type)
     return tokError("unexpected type in metadata definition");
 
+  auto DistinctLoc = Lex.getLoc();
   bool IsDistinct = EatIfPresent(lltok::kw_distinct);
   if (Lex.getKind() == lltok::MetadataVar) {
-    if (parseSpecializedMDNode(Init, IsDistinct))
+    if (parseSpecializedMDNode(Init, IsDistinct, DistinctLoc))
       return true;
   } else if (parseToken(lltok::exclaim, "Expected '!' here") ||
              parseMDTuple(Init, IsDistinct))
     return true;
 
   // See if this was forward referenced, if so, handle it.
   auto FI = ForwardRefMDNodes.find(MetadataID);
   if (FI != ForwardRefMDNodes.end()) {
bool LLParser::parseMDField(StringRef Name, FieldTy &Result) {
   if (Result.Seen)
     return tokError("field '" + Name + "' cannot be specified more than once");
 
   LocTy Loc = Lex.getLoc();
   Lex.Lex();
   return parseMDField(Loc, Name, Result);
 }
 
-bool LLParser::parseSpecializedMDNode(MDNode *&N, bool IsDistinct) {
+bool LLParser::parseSpecializedMDNode(MDNode *&N, bool IsDistinct,
+                                      LocTy DistinctLoc) {
   assert(Lex.getKind() == lltok::MetadataVar && "Expected metadata type name");
 
-#define HANDLE_SPECIALIZED_MDNODE_LEAF(CLASS)                                  \
+#define HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(CLASS)                        \
   if (Lex.getStrVal() == #CLASS)                                               \
     return parse##CLASS(N, IsDistinct);
+#define HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUED(CLASS)                          \
+  if (Lex.getStrVal() == #CLASS) {                                             \
+    if (IsDistinct)                                                            \
+      return error(DistinctLoc, "'distinct' not allowed for !" #CLASS);        \
+    return parse##CLASS(N, IsDistinct);                                        \
+  }
+#define HANDLE_SPECIALIZED_MDNODE_LEAF_DISTINCT(CLASS)                         \
+  if (Lex.getStrVal() == #CLASS) {                                             \
+    if (!IsDistinct)                                                           \
+      return error(DistinctLoc, "missing 'distinct', required for !" #CLASS);  \
+    return parse##CLASS(N, IsDistinct);                                        \
+  }
 #include "llvm/IR/Metadata.def"
 
   return tokError("expected metadata type");
 }
 
 #define DECLARE_FIELD(NAME, TYPE, INIT) TYPE NAME INIT
 #define NOP_FIELD(NAME, TYPE, INIT)
 #define REQUIRE_FIELD(NAME, TYPE, INIT)                                        \

 /// parseDICompileUnit:
 ///   ::= !DICompileUnit(language: DW_LANG_C99, file: !0, producer: "clang",
 ///                      isOptimized: true, flags: "-O2", runtimeVersion: 1,
 ///                      splitDebugFilename: "abc.debug",
 ///                      emissionKind: FullDebug, enums: !1, retainedTypes: !2,
 ///                      globals: !4, imports: !5, macros: !6, dwoId: 0x0abcd,
 ///                      sysroot: "/", sdk: "MacOSX.sdk")
 bool LLParser::parseDICompileUnit(MDNode *&Result, bool IsDistinct) {
-  if (!IsDistinct)
-    return Lex.Error("missing 'distinct', required for !DICompileUnit");
-
 #define VISIT_MD_FIELDS(OPTIONAL, REQUIRED)                                    \
   REQUIRED(language, DwarfLangField, );                                        \
   REQUIRED(file, MDField, (/* AllowNull */ false));                            \
   OPTIONAL(producer, MDStringField, );                                         \
   OPTIONAL(isOptimized, MDBoolField, );                                        \
   OPTIONAL(flags, MDStringField, );                                            \
   OPTIONAL(runtimeVersion, MDUnsignedField, (0, UINT32_MAX));                  \
   OPTIONAL(splitDebugFilename, MDStringField, );                               \
#undef VISIT_MD_FIELDS
   Result = GET_OR_DISTINCT(DILabel,
                            (Context, scope.Val, name.Val, file.Val, line.Val));
   return false;
 }
 
 /// parseDIExpression:
 ///   ::= !DIExpression(0, 7, -1)
 bool LLParser::parseDIExpression(MDNode *&Result, bool IsDistinct) {
+  assert(!IsDistinct && "DIExpression must not be distinct");
   assert(Lex.getKind() == lltok::MetadataVar && "Expected metadata type name");
   Lex.Lex();
 
   if (parseToken(lltok::lparen, "expected '(' here"))
     return true;
 
   SmallVector<uint64_t, 8> Elements;
   if (Lex.getKind() != lltok::rparen)
do {
         return tokError("element too large, limit is " + Twine(UINT64_MAX));
       Elements.push_back(U.getZExtValue());
       Lex.Lex();
     } while (EatIfPresent(lltok::comma));
 
   if (parseToken(lltok::rparen, "expected ')' here"))
     return true;
 
-  Result = GET_OR_DISTINCT(DIExpression, (Context, Elements));
+  Result = DIExpression::get(Context, Elements);
   return false;
 }
 
-bool LLParser::parseDIArgList(MDNode *&Result, bool IsDistinct) {
-  return parseDIArgList(Result, IsDistinct, nullptr);
-}
 /// ParseDIArgList:
 ///   ::= !DIArgList(i32 7, i64 %0)
-bool LLParser::parseDIArgList(MDNode *&Result, bool IsDistinct,
+bool LLParser::parseDIArgList(Metadata *&Result, bool IsDistinct,
                               PerFunctionState *PFS) {
+  assert(!IsDistinct && "DIArgList must not be distinct");
   assert(PFS && "Expected valid function state");
   assert(Lex.getKind() == lltok::MetadataVar && "Expected metadata type name");
   Lex.Lex();
 
   if (parseToken(lltok::lparen, "expected '(' here"))
     return true;
 
   SmallVector<ValueAsMetadata *, 4> Args;
   if (Lex.getKind() != lltok::rparen)
     do {
       Metadata *MD;
       if (parseValueAsMetadata(MD, "expected value-as-metadata operand", PFS))
         return true;
       Args.push_back(dyn_cast<ValueAsMetadata>(MD));
     } while (EatIfPresent(lltok::comma));
 
   if (parseToken(lltok::rparen, "expected ')' here"))
     return true;
 
-  Result = GET_OR_DISTINCT(DIArgList, (Context, Args));
+  Result = DIArgList::get(Context, Args);
   return false;
 }
 
 /// parseDIGlobalVariableExpression:
 ///   ::= !DIGlobalVariableExpression(var: !0, expr: !1)
 bool LLParser::parseDIGlobalVariableExpression(MDNode *&Result,
                                                bool IsDistinct) {
 #define VISIT_MD_FIELDS(OPTIONAL, REQUIRED)                                    \

 ///  ::= i32 @global
 ///  ::= i32 7
 ///  ::= !42
 ///  ::= !{...}
 ///  ::= !"string"
 ///  ::= !DILocation(...)
 bool LLParser::parseMetadata(Metadata *&MD, PerFunctionState *PFS) {
   if (Lex.getKind() == lltok::MetadataVar) {
+    if (Lex.getStrVal() == "DIArgList")
+      return parseDIArgList(MD, false, PFS);
     MDNode *N;
-    // DIArgLists are a special case, as they are a list of ValueAsMetadata and
-    // so parsing this requires a Function State.
-    if (Lex.getStrVal() == "DIArgList") {
-      if (parseDIArgList(N, false, PFS))
-        return true;
-    } else if (parseSpecializedMDNode(N)) {
+    if (parseSpecializedMDNode(N))
       return true;
-    }
     MD = N;
     return false;
   }
 
   // ValueAsMetadata:
   // <type> <value>
   if (Lex.getKind() != lltok::exclaim)
     return parseValueAsMetadata(MD, "expected metadata operand", PFS);

llvm/lib/Bitcode/Reader/MetadataLoader.cpp

for (auto &BB : F)
               SmallVector<uint64_t, 8> Ops;
               Ops.append(std::next(DIExpr->elements_begin()),
                          DIExpr->elements_end());
               DDI->setExpression(DIExpression::get(Context, Ops));
             }
   }
 
   /// Upgrade the expression from previous versions.
-  Error upgradeDIExpression(uint64_t FromVersion,
+  Error upgradeDIExpression(uint64_t FromVersion, bool &IsDistinct,
                             MutableArrayRef<uint64_t> &Expr,
                             SmallVectorImpl<uint64_t> &Buffer) {
     auto N = Expr.size();
     switch (FromVersion) {
     default:
       return error("Invalid record");
     case 0:
       if (N >= 3 && Expr[N - 3] == dwarf::DW_OP_bit_piece)
case 2: {
 
         // Continue with remaining elements.
         SubExpr = SubExpr.slice(HistoricSize);
       }
       Expr = MutableArrayRef<uint64_t>(Buffer);
       LLVM_FALLTHROUGH;
     }
     case 3:
+      IsDistinct = false;
+      LLVM_FALLTHROUGH;
+    case 4:
       // Up-to-date!
       break;
     }
 
     return Error::success();
   }
 
   void upgradeDebugInfo() {
void MetadataLoader::MetadataLoaderImpl::resolveForwardRefsAndPlaceholders(
   // with the final node they refer to.
   Placeholders.flush(MetadataList);
 }
 
 Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
     SmallVectorImpl<uint64_t> &Record, unsigned Code,
     PlaceholderQueue &Placeholders, StringRef Blob, unsigned &NextMetadataNo) {
 
   bool IsDistinct = false;
   auto getMD = [&](unsigned ID) -> Metadata * {
     if (ID < MDStringRef.size())
       return lazyLoadOneMDString(ID);
     if (!IsDistinct) {
       if (auto *MD = MetadataList.lookup(ID))
         return MD;
       // If lazy-loading is enabled, we try recursively to load the operand
       // instead of creating a temporary.
MetadataList.assignValue(
         NextMetadataNo);
     NextMetadataNo++;
     break;
   }
   case bitc::METADATA_EXPRESSION: {
     if (Record.size() < 1)
       return error("Invalid record");
 
     IsDistinct = Record[0] & 1;
     uint64_t Version = Record[0] >> 1;
     auto Elts = MutableArrayRef<uint64_t>(Record).slice(1);
 
     SmallVector<uint64_t, 6> Buffer;
-    if (Error Err = upgradeDIExpression(Version, Elts, Buffer))
+    if (Error Err = upgradeDIExpression(Version, IsDistinct, Elts, Buffer))
       return Err;
 
-    MetadataList.assignValue(GET_OR_DISTINCT(DIExpression, (Context, Elts)),
-                             NextMetadataNo);
+    if (IsDistinct)
+      return error("Invalid record");
+
+    MetadataList.assignValue(DIExpression::get(Context, Elts), NextMetadataNo);
     NextMetadataNo++;
     break;
   }
   case bitc::METADATA_GLOBAL_VAR_EXPR: {
     if (Record.size() != 3)
       return error("Invalid record");
 
     IsDistinct = Record[0];

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

private:
   void writeDICommonBlock(const DICommonBlock *N,
                           SmallVectorImpl<uint64_t> &Record, unsigned Abbrev);
   void writeDINamespace(const DINamespace *N, SmallVectorImpl<uint64_t> &Record,
                         unsigned Abbrev);
   void writeDIMacro(const DIMacro *N, SmallVectorImpl<uint64_t> &Record,
                     unsigned Abbrev);
   void writeDIMacroFile(const DIMacroFile *N, SmallVectorImpl<uint64_t> &Record,
                         unsigned Abbrev);
-  void writeDIArgList(const DIArgList *N, SmallVectorImpl<uint64_t> &Record,
-                      unsigned Abbrev);
+  void writeDIArgList(const DIArgList *N, SmallVectorImpl<uint64_t> &Record);
   void writeDIModule(const DIModule *N, SmallVectorImpl<uint64_t> &Record,
                      unsigned Abbrev);
   void writeDITemplateTypeParameter(const DITemplateTypeParameter *N,
                                     SmallVectorImpl<uint64_t> &Record,
                                     unsigned Abbrev);
   void writeDITemplateValueParameter(const DITemplateValueParameter *N,
                                      SmallVectorImpl<uint64_t> &Record,
                                      unsigned Abbrev);
void ModuleBitcodeWriter::writeDIFile(const DIFile *N,
 
   Stream.EmitRecord(bitc::METADATA_FILE, Record, Abbrev);
   Record.clear();
 }
 
 void ModuleBitcodeWriter::writeDICompileUnit(const DICompileUnit *N,
                                              SmallVectorImpl<uint64_t> &Record,
                                              unsigned Abbrev) {
-  assert(N->isDistinct() && "Expected distinct compile units");
   Record.push_back(/* IsDistinct */ true);
   Record.push_back(N->getSourceLanguage());
   Record.push_back(VE.getMetadataOrNullID(N->getFile()));
   Record.push_back(VE.getMetadataOrNullID(N->getRawProducer()));
   Record.push_back(N->isOptimized());
   Record.push_back(VE.getMetadataOrNullID(N->getRawFlags()));
   Record.push_back(N->getRuntimeVersion());
   Record.push_back(VE.getMetadataOrNullID(N->getRawSplitDebugFilename()));
void ModuleBitcodeWriter::writeDIMacroFile(const DIMacroFile *N,
   Record.push_back(VE.getMetadataOrNullID(N->getFile()));
   Record.push_back(VE.getMetadataOrNullID(N->getElements().get()));
 
   Stream.EmitRecord(bitc::METADATA_MACRO_FILE, Record, Abbrev);
   Record.clear();
 }
 
 void ModuleBitcodeWriter::writeDIArgList(const DIArgList *N,
-                                         SmallVectorImpl<uint64_t> &Record,
-                                         unsigned Abbrev) {
+                                         SmallVectorImpl<uint64_t> &Record) {
   Record.reserve(N->getArgs().size());
   for (ValueAsMetadata *MD : N->getArgs())
     Record.push_back(VE.getMetadataID(MD));
 
-  Stream.EmitRecord(bitc::METADATA_ARG_LIST, Record, Abbrev);
+  Stream.EmitRecord(bitc::METADATA_ARG_LIST, Record);
   Record.clear();
 }
 
 void ModuleBitcodeWriter::writeDIModule(const DIModule *N,
                                         SmallVectorImpl<uint64_t> &Record,
                                         unsigned Abbrev) {
   Record.push_back(N->isDistinct());
   for (auto &I : N->operands())
void ModuleBitcodeWriter::writeDILabel(
   Stream.EmitRecord(bitc::METADATA_LABEL, Record, Abbrev);
   Record.clear();
 }
 
 void ModuleBitcodeWriter::writeDIExpression(const DIExpression *N,
                                             SmallVectorImpl<uint64_t> &Record,
                                             unsigned Abbrev) {
   Record.reserve(N->getElements().size() + 1);
-  const uint64_t Version = 3 << 1;
+  const uint64_t Version = 4 << 1;
   Record.push_back((uint64_t)N->isDistinct() | Version);
   Record.append(N->elements_begin(), N->elements_end());
 
   Stream.EmitRecord(bitc::METADATA_EXPRESSION, Record, Abbrev);
   Record.clear();
 }
 
 void ModuleBitcodeWriter::writeDIGlobalVariableExpression(

 #include "llvm/IR/Metadata.def"
 
   for (const Metadata *MD : MDs) {
     if (IndexPos)
       IndexPos->push_back(Stream.GetCurrentBitNo());
     if (const MDNode *N = dyn_cast<MDNode>(MD)) {
       assert(N->isResolved() && "Expected forward references to be resolved");
 
+#ifndef NDEBUG
+      switch (N->getMetadataID()) {
+#define HANDLE_MDNODE_LEAF_UNIQUED(CLASS)                                      \
+  case Metadata::CLASS##Kind:                                                  \
+    assert(!N->isDistinct() && "Expected non-distinct " #CLASS);               \
+    break;
+#define HANDLE_MDNODE_LEAF_DISTINCT(CLASS)                                     \
+  case Metadata::CLASS##Kind:                                                  \
+    assert(N->isDistinct() && "Expected distinct " #CLASS);                    \
+    break;
+#include "llvm/IR/Metadata.def"
+      }
+#endif
+
       switch (N->getMetadataID()) {
       default:
         llvm_unreachable("Invalid MDNode subclass");
 #define HANDLE_MDNODE_LEAF(CLASS)                                              \
   case Metadata::CLASS##Kind:                                                  \
     if (MDAbbrevs)                                                             \
       write##CLASS(cast<CLASS>(N), Record,                                     \
                    (*MDAbbrevs)[MetadataAbbrev::CLASS##AbbrevID]);             \
     else                                                                       \
       write##CLASS(cast<CLASS>(N), Record, CLASS##Abbrev);                     \
     continue;
 #include "llvm/IR/Metadata.def"
       }
     }
+    if (auto *D = dyn_cast<DIArgList>(MD))
+      writeDIArgList(D, Record);
+    else
       writeValueAsMetadata(cast<ValueAsMetadata>(MD), Record);
   }
 }
 
 void ModuleBitcodeWriter::writeModuleMetadata() {
   if (!VE.hasMDs() && M.named_metadata_empty())
     return;
 
   Stream.EnterSubblock(bitc::METADATA_BLOCK_ID, 4);

llvm/lib/CodeGen/MIRParser/MIParser.cpp

 }
 
 bool MIParser::parseStandaloneMDNode(MDNode *&Node) {
   lex();
   if (Token.is(MIToken::exclaim)) {
     if (parseMDNode(Node))
       return true;
   } else if (Token.is(MIToken::md_diexpr)) {
+    // FIXME: This should be driven off of the UNIQUED property in Metadata.def
     if (parseDIExpression(Node))
       return true;
   } else if (Token.is(MIToken::md_dilocation)) {
     if (parseDILocation(Node))
       return true;
   } else
     return error("expected a metadata node");
   if (Token.isNot(MIToken::Eof))

 }
 
 bool MIParser::parseMetadataOperand(MachineOperand &Dest) {
   MDNode *Node = nullptr;
   if (Token.is(MIToken::exclaim)) {
     if (parseMDNode(Node))
       return true;
   } else if (Token.is(MIToken::md_diexpr)) {
+    // FIXME: This should be driven off of the UNIQUED property in Metadata.def
     if (parseDIExpression(Node))
       return true;
   }
   Dest = MachineOperand::CreateMetadata(Node);
   return false;
 }
 
 bool MIParser::parseCFIOffset(int &Offset) {

llvm/lib/IR/AsmWriter.cpp

void SlotTracker::CreateFunctionSlot(const Value *V) {
   ST_DEBUG("  Inserting value [" << V->getType() << "] = " << V << " slot=" <<
            DestSlot << " [o]\n");
 }
 
 /// CreateModuleSlot - Insert the specified MDNode* into the slot table.
 void SlotTracker::CreateMetadataSlot(const MDNode *N) {
   assert(N && "Can't insert a null Value into SlotTracker!");
 
-  // Don't make slots for DIExpressions or DIArgLists. We just print them inline
-  // everywhere.
-  if (isa<DIExpression>(N) || isa<DIArgList>(N))
+  // Don't make slots for uniqued nodes. We just print them inline everywhere.
+#define HANDLE_MDNODE_LEAF_UNIQUED(CLASS)                                      \
+  if (isa<CLASS>(N))                                                           \
     return;
+#include "llvm/IR/Metadata.def"
 
   unsigned DestSlot = mdnNext;
   if (!mdnMap.insert(std::make_pair(N, DestSlot)).second)
     return;
   ++mdnNext;
 
   // Recursively add any MDNodes referenced by operands.
   for (unsigned i = 0, e = N->getNumOperands(); i != e; ++i)
static void writeDIExpression(raw_ostream &Out, const DIExpression *N,
   } else {
     for (const auto &I : N->getElements())
       Out << FS << I;
   }
   Out << ")";
 }
 
 static void writeDIArgList(raw_ostream &Out, const DIArgList *N,
-                           AsmWriterContext &WriterCtx,
-                           bool FromValue = false) {
-  assert(FromValue &&
-         "Unexpected DIArgList metadata outside of value argument");
+                           AsmWriterContext &WriterCtx) {
   Out << "!DIArgList(";
   FieldSeparator FS;
   MDFieldPrinter Printer(Out, WriterCtx);
   for (Metadata *Arg : N->getArgs()) {
     Out << FS;
     WriteAsOperandInternal(Out, Arg, WriterCtx, true);
   }
   Out << ")";
if (Slot != -1)
     Out << Prefix << Slot;
   else
     Out << "<badref>";
 }
 
 static void WriteAsOperandInternal(raw_ostream &Out, const Metadata *MD,
                                    AsmWriterContext &WriterCtx,
                                    bool FromValue) {
-  // Write DIExpressions and DIArgLists inline when used as a value. Improves
-  // readability of debug info intrinsics.
-  if (const DIExpression *Expr = dyn_cast<DIExpression>(MD)) {
-    writeDIExpression(Out, Expr, WriterCtx);
-    return;
+  assert((FromValue || !(isa<LocalAsMetadata>(MD) || isa<DIArgList>(MD))) &&
+         "Unexpected function-local metadata outside of value argument");
+
+  // Write uniqued MDNodes inline when used as a value.
+#define HANDLE_MDNODE_LEAF_UNIQUED(CLASS)                                      \
+  if (const CLASS *N = dyn_cast<CLASS>(MD)) {                                  \
+    write##CLASS(Out, N, WriterCtx);                                           \
+    return;                                                                    \
   }
-  if (const DIArgList *ArgList = dyn_cast<DIArgList>(MD)) {
-    writeDIArgList(Out, ArgList, WriterCtx, FromValue);
+#include "llvm/IR/Metadata.def"
+  if (const DIArgList *N = dyn_cast<DIArgList>(MD)) {
+    writeDIArgList(Out, N, WriterCtx);
     return;
   }
 
   if (const MDNode *N = dyn_cast<MDNode>(MD)) {
     std::unique_ptr<SlotTracker> MachineStorage;
     SaveAndRestore<SlotTracker *> SARMachine(WriterCtx.Machine);
     if (!WriterCtx.Machine) {
       MachineStorage = std::make_unique<SlotTracker>(WriterCtx.Context);
if (const MDString *MDS = dyn_cast<MDString>(MD)) {
     Out << "!\"";
     printEscapedString(MDS->getString(), Out);
     Out << '"';
     return;
   }
 
   auto *V = cast<ValueAsMetadata>(MD);
   assert(WriterCtx.TypePrinter && "TypePrinter required for metadata values");
-  assert((FromValue || !isa<LocalAsMetadata>(V)) &&
-         "Unexpected function-local metadata outside of value argument");
-
   WriterCtx.TypePrinter->print(V->getValue()->getType(), Out);
   Out << ' ';
   WriteAsOperandInternal(Out, V->getValue(), WriterCtx);
 }
 
 namespace {
 
 class AssemblyWriter {

 void AssemblyWriter::printNamedMDNode(const NamedMDNode *NMD) {
   Out << '!';
   printMetadataIdentifier(NMD->getName(), Out);
   Out << " = !{";
   for (unsigned i = 0, e = NMD->getNumOperands(); i != e; ++i) {
     if (i)
       Out << ", ";
 
-    // Write DIExpressions inline.
+    // Write UNIQUED nodes inline.
     // FIXME: Ban DIExpressions in NamedMDNodes, they will serve no purpose.
     MDNode *Op = NMD->getOperand(i);
     assert(!isa<DIArgList>(Op) &&
            "DIArgLists should not appear in NamedMDNodes");
-    if (auto *Expr = dyn_cast<DIExpression>(Op)) {
-      writeDIExpression(Out, Expr, AsmWriterContext::getEmpty());
-      continue;
-    }
+#define HANDLE_MDNODE_LEAF_UNIQUED(CLASS)                                      \
+  if (auto *N = dyn_cast<CLASS>(Op)) {                                         \
+    write##CLASS(Out, N, AsmWriterContext::getEmpty());                        \
+    continue;                                                                  \
+#include "llvm/IR/Metadata.def"
 
     int Slot = Machine.getMetadataSlot(Op);
     if (Slot == -1)
       Out << "<badref>";
     else
       Out << '!' << Slot;
   }
   Out << "}\n";

 
 static void printMetadataImpl(raw_ostream &ROS, const Metadata &MD,
                               ModuleSlotTracker &MST, const Module *M,
                               bool OnlyAsOperand, bool PrintAsTree = false) {
   formatted_raw_ostream OS(ROS);
 
   TypePrinting TypePrinter(M);
 
   std::unique_ptr<AsmWriterContext> WriterCtx;
   if (PrintAsTree && !OnlyAsOperand)
     WriterCtx = std::make_unique<MDTreeAsmWriterContext>(
         &TypePrinter, MST.getMachine(), M, OS, &MD);
   else
     WriterCtx =
         std::make_unique<AsmWriterContext>(&TypePrinter, MST.getMachine(), M);
 
   WriteAsOperandInternal(OS, &MD, *WriterCtx, /* FromValue */ true);
 
   auto *N = dyn_cast<MDNode>(&MD);
-  if (OnlyAsOperand || !N || isa<DIExpression>(MD) || isa<DIArgList>(MD))
+  if (OnlyAsOperand || !N) {
     return;
+  }
+  // Uniqued MDNodes are always treated as if OnlyAsOperand, as they are
+  // printed inline.
+#define HANDLE_MDNODE_LEAF_UNIQUED(CLASS)                                      \
+  if (isa<CLASS>(MD))                                                          \
+    return;
+#include "llvm/IR/Metadata.def"
 
   OS << " = ";
   WriteMDNodeBodyInternal(OS, N, *WriterCtx);
 }
 
 void Metadata::printAsOperand(raw_ostream &OS, const Module *M) const {
   ModuleSlotTracker MST(M, isa<MDNode>(this));
   printMetadataImpl(OS, *this, MST, M, /* OnlyAsOperand */ true);

llvm/lib/IR/DebugInfoMetadata.cpp

 #include "llvm/IR/DebugInfoMetadata.h"
 #include "LLVMContextImpl.h"
 #include "MetadataImpl.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/IR/DIBuilder.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/Instructions.h"
+#include "llvm/IR/Metadata.h"
 
 #include <numeric>
 
 using namespace llvm;
 
 namespace llvm {
 // Use FS-AFDO discriminator.
 cl::opt<bool> EnableFSDiscriminator(
DILabel *DILabel::getImpl(LLVMContext &Context, Metadata *Scope, MDString *Name,
   DEFINE_GETIMPL_LOOKUP(DILabel, (Scope, Name, File, Line));
   Metadata *Ops[] = {Scope, Name, File};
   DEFINE_GETIMPL_STORE(DILabel, (Line), Ops);
 }
 
 DIExpression *DIExpression::getImpl(LLVMContext &Context,
                                     ArrayRef<uint64_t> Elements,
                                     StorageType Storage, bool ShouldCreate) {
+  assert(Storage != Distinct && "DIExpression cannot be distinct");
   DEFINE_GETIMPL_LOOKUP(DIExpression, (Elements));
   DEFINE_GETIMPL_STORE_NO_OPS(DIExpression, (Elements));
 }
 
 unsigned DIExpression::ExprOperand::getSize() const {
   uint64_t Op = getOp();
 
   if (Op >= dwarf::DW_OP_breg0 && Op <= dwarf::DW_OP_breg31)
DIMacroFile *DIMacroFile::getImpl(LLVMContext &Context, unsigned MIType,
                                   bool ShouldCreate) {
   DEFINE_GETIMPL_LOOKUP(DIMacroFile, (MIType, Line, File, Elements));
   Metadata *Ops[] = {File, Elements};
   DEFINE_GETIMPL_STORE(DIMacroFile, (MIType, Line), Ops);
 }
 
 DIArgList *DIArgList::getImpl(LLVMContext &Context,
                               ArrayRef<ValueAsMetadata *> Args,
-                              StorageType Storage, bool ShouldCreate) {
-  DEFINE_GETIMPL_LOOKUP(DIArgList, (Args));
-  DEFINE_GETIMPL_STORE_NO_OPS(DIArgList, (Args));
+                              bool ShouldCreate) {
+  if (auto *N = getUniqued(Context.pImpl->DIArgLists, DIArgListInfo(Args)))
+    return N;
+  if (!ShouldCreate)
+    return nullptr;
+  return *Context.pImpl->DIArgLists.insert(new DIArgList(Context, Args)).first;
 }
 
+template <class T, class InfoT>
+static T *uniquifyImpl(T *N, DenseSet<T *, InfoT> &Store) {
+  if (T *U = getUniqued(Store, N))
+    return U;
+
+  Store.insert(N);
+  return N;
+}
 void DIArgList::handleChangedOperand(void *Ref, Metadata *New) {
   ValueAsMetadata **OldVMPtr = static_cast<ValueAsMetadata **>(Ref);
   assert((!New || isa<ValueAsMetadata>(New)) &&
-         "DIArgList must be passed a ValueAsMetadata");
+         "DIArgList arguments must be ValueAsMetadata");
   untrack();
-  bool Uniq = isUniqued();
-  if (Uniq) {
-    // We need to update the uniqueness once the Args are updated since they
-    // form the key to the DIArgLists store.
-    eraseFromStore();
-  }
+  getContext().pImpl->DIArgLists.erase(this);
   ValueAsMetadata *NewVM = cast_or_null<ValueAsMetadata>(New);
   for (ValueAsMetadata *&VM : Args) {
     if (&VM == OldVMPtr) {
       if (NewVM)
         VM = NewVM;
       else
         VM = ValueAsMetadata::get(UndefValue::get(VM->getValue()->getType()));
     }
   }
-  if (Uniq) {
-    if (uniquify() != this)
-      storeDistinctInContext();
+  auto *Uniqued = uniquifyImpl(this, getContext().pImpl->DIArgLists);
+  if (Uniqued != this) {
+    ReplaceableMetadataImpl::replaceAllUsesWith(Uniqued);
+    for (ValueAsMetadata *&VAM : Args)
+      VAM = nullptr;
+    delete this;
+    return;
   }
   track();
 }
 void DIArgList::track() {
   for (ValueAsMetadata *&VAM : Args)
     if (VAM)
       MetadataTracking::track(&VAM, *VAM, *this);
 }
 void DIArgList::untrack() {
   for (ValueAsMetadata *&VAM : Args)
     if (VAM)
       MetadataTracking::untrack(&VAM, *VAM);
 }
 void DIArgList::dropAllReferences() {
   untrack();
   Args.clear();
-  MDNode::dropAllReferences();
 }

llvm/lib/IR/LLVMContextImpl.h

return MIType == RHS->getMacinfoType() && Line == RHS->getLine() &&
            File == RHS->getRawFile() && Elements == RHS->getRawElements();
   }
 
   unsigned getHashValue() const {
     return hash_combine(MIType, Line, File, Elements);
   }
 };
 
-template <> struct MDNodeKeyImpl<DIArgList> {
+struct DIArgListInfo {
+  using NodeTy = DIArgList;
+  using KeyTy = DIArgListInfo;
   ArrayRef<ValueAsMetadata *> Args;
 
-  MDNodeKeyImpl(ArrayRef<ValueAsMetadata *> Args) : Args(Args) {}
-  MDNodeKeyImpl(const DIArgList *N) : Args(N->getArgs()) {}
+  static inline NodeTy *getEmptyKey() {
+    return DenseMapInfo<NodeTy *>::getEmptyKey();
+  }
+
+  static inline NodeTy *getTombstoneKey() {
+    return DenseMapInfo<NodeTy *>::getTombstoneKey();
+  }
+
+  static unsigned getHashValue(const KeyTy &Key) { return Key.getHashValue(); }
+
+  static unsigned getHashValue(const NodeTy *N) {
+    return KeyTy(N).getHashValue();
+  }
+
+  static bool isEqual(const KeyTy &LHS, const NodeTy *RHS) {
+    if (RHS == getEmptyKey() || RHS == getTombstoneKey())
+      return false;
+    return LHS.isKeyOf(RHS);
+  }
+
+  static bool isEqual(const NodeTy *LHS, const NodeTy *RHS) {
+    return LHS == RHS;
+  }
+
+  DIArgListInfo(ArrayRef<ValueAsMetadata *> Args) : Args(Args) {}
+  DIArgListInfo(const DIArgList *N) : Args(N->getArgs()) {}
 
   bool isKeyOf(const DIArgList *RHS) const { return Args == RHS->getArgs(); }
 
   unsigned getHashValue() const {
     return hash_combine_range(Args.begin(), Args.end());
   }
 };
 
public:
 
   FoldingSet<AttributeImpl> AttrsSet;
   FoldingSet<AttributeListImpl> AttrsLists;
   FoldingSet<AttributeSetNode> AttrsSetNodes;
 
   StringMap<MDString, BumpPtrAllocator> MDStringCache;
   DenseMap<Value *, ValueAsMetadata *> ValuesAsMetadata;
   DenseMap<Metadata *, MetadataAsValue *> MetadataAsValues;
+  DenseSet<DIArgList *, DIArgListInfo> DIArgLists;
 
   DenseMap<const Value *, ValueName *> ValueNames;
 
 #define HANDLE_MDNODE_LEAF_UNIQUABLE(CLASS)                                    \
   DenseSet<CLASS *, CLASS##Info> CLASS##s;
+#define HANDLE_MDNODE_LEAF_UNIQUED(CLASS) HANDLE_MDNODE_LEAF_UNIQUABLE(CLASS)
 #include "llvm/IR/Metadata.def"
 
   // Optional map for looking up composite types by identifier.
   Optional<DenseMap<const MDString *, DICompositeType *>> DITypeMap;
 
   // MDNodes may be uniqued or not uniqued.  When they're not uniqued, they
   // aren't in the MDNodeSet, but they're still shared between objects, so no
   // one object can destroy them.  Keep track of them here so we can delete

llvm/lib/IR/LLVMContextImpl.cpp

if (auto *AL = dyn_cast<DIArgList>(I)) {
       AL->dropAllReferences();
       continue;
     }
     I->dropAllReferences();
   }
 #define HANDLE_MDNODE_LEAF_UNIQUABLE(CLASS)                                    \
   for (auto *I : CLASS##s)                                                     \
     I->dropAllReferences();
+#define HANDLE_MDNODE_LEAF_UNIQUED(CLASS) HANDLE_MDNODE_LEAF_UNIQUABLE(CLASS)
 #include "llvm/IR/Metadata.def"
 
   // Also drop references that come from the Value bridges.
   for (auto &Pair : ValuesAsMetadata)
     Pair.second->dropUsers();
   for (auto &Pair : MetadataAsValues)
     Pair.second->dropUse();
 
   // Destroy MDNodes.
   for (MDNode *I : DistinctMDNodes)
     I->deleteAsSubclass();
 #define HANDLE_MDNODE_LEAF_UNIQUABLE(CLASS)                                    \
   for (CLASS * I : CLASS##s)                                                   \
     delete I;
+#define HANDLE_MDNODE_LEAF_UNIQUED(CLASS) HANDLE_MDNODE_LEAF_UNIQUABLE(CLASS)
 #include "llvm/IR/Metadata.def"
 
   // Free the constants.
   for (auto *I : ExprConstants)
     I->dropAllReferences();
   for (auto *I : ArrayConstants)
     I->dropAllReferences();
   for (auto *I : StructConstants)

llvm/lib/IR/Metadata.cpp

if (!OwnerMD)
       continue;
     if (OwnerMD->isResolved())
       continue;
     OwnerMD->decrementUnresolvedOperandCount();
   }
 }
 
 ReplaceableMetadataImpl *ReplaceableMetadataImpl::getOrCreate(Metadata &MD) {
+  if (auto *N = dyn_cast<DIArgList>(&MD))
+    return N;
   if (auto *N = dyn_cast<MDNode>(&MD))
     return N->isResolved() ? nullptr : N->Context.getOrCreateReplaceableUses();
   return dyn_cast<ValueAsMetadata>(&MD);
 }
 
 ReplaceableMetadataImpl *ReplaceableMetadataImpl::getIfExists(Metadata &MD) {
+  if (auto *N = dyn_cast<DIArgList>(&MD))
+    return N;
   if (auto *N = dyn_cast<MDNode>(&MD))
     return N->isResolved() ? nullptr : N->Context.getReplaceableUses();
   return dyn_cast<ValueAsMetadata>(&MD);
 }
 
 bool ReplaceableMetadataImpl::isReplaceable(const Metadata &MD) {
+  if (auto *N = dyn_cast<DIArgList>(&MD))
+    return true;
   if (auto *N = dyn_cast<MDNode>(&MD))
     return !N->isResolved();
   return isa<ValueAsMetadata>(&MD);
 }
 
 static DISubprogram *getLocalFunctionMetadata(Value *V) {
   assert(V && "Expected value");
   if (auto *A = dyn_cast<Argument>(V)) {
MDNode *MDNode::replaceWithPermanentImpl() {
   switch (getMetadataID()) {
   default:
     // If this type isn't uniquable, replace with a distinct node.
     return replaceWithDistinctImpl();
 
 #define HANDLE_MDNODE_LEAF_UNIQUABLE(CLASS)                                    \
   case CLASS##Kind:                                                            \
     break;
+#define HANDLE_MDNODE_LEAF_UNIQUED(CLASS) HANDLE_MDNODE_LEAF_UNIQUABLE(CLASS)
 #include "llvm/IR/Metadata.def"
   }
 
   // Even if this type is uniquable, self-references have to be distinct.
   if (hasSelfReference(this))
     return replaceWithDistinctImpl();
   return replaceWithUniquedImpl();
 }

 #define HANDLE_MDNODE_LEAF_UNIQUABLE(CLASS)                                    \
   case CLASS##Kind: {                                                          \
     CLASS *SubclassThis = cast<CLASS>(this);                                   \
     std::integral_constant<bool, HasCachedHash<CLASS>::value>                  \
         ShouldRecalculateHash;                                                 \
     dispatchRecalculateHash(SubclassThis, ShouldRecalculateHash);              \
     return uniquifyImpl(SubclassThis, getContext().pImpl->CLASS##s);           \
   }
+#define HANDLE_MDNODE_LEAF_UNIQUED(CLASS) HANDLE_MDNODE_LEAF_UNIQUABLE(CLASS)
 #include "llvm/IR/Metadata.def"
   }
 }
 
 void MDNode::eraseFromStore() {
   switch (getMetadataID()) {
   default:
     llvm_unreachable("Invalid or non-uniquable subclass of MDNode");
 #define HANDLE_MDNODE_LEAF_UNIQUABLE(CLASS)                                    \
   case CLASS##Kind:                                                            \
     getContext().pImpl->CLASS##s.erase(cast<CLASS>(this));                     \
     break;
+#define HANDLE_MDNODE_LEAF_UNIQUED(CLASS) HANDLE_MDNODE_LEAF_UNIQUABLE(CLASS)
 #include "llvm/IR/Metadata.def"
   }
 }
 
 MDTuple *MDTuple::getImpl(LLVMContext &Context, ArrayRef<Metadata *> MDs,
                           StorageType Storage, bool ShouldCreate) {
   unsigned Hash = 0;
   if (Storage == Uniqued) {

llvm/lib/IR/TypeFinder.cpp

 }
 
 /// incorporateValue - This method is used to walk operand lists finding types
 /// hiding in constant expressions and other operands that won't be walked in
 /// other ways.  GlobalValues, basic blocks, instructions, and inst operands are
 /// all explicitly enumerated.
 void TypeFinder::incorporateValue(const Value *V) {
   if (const auto *M = dyn_cast<MetadataAsValue>(V)) {
+    if (const auto *AL = dyn_cast<DIArgList>(M->getMetadata())) {
+      for (auto *Arg : AL->getArgs())
+        incorporateValue(Arg->getValue());
+      return;
+    }
     if (const auto *N = dyn_cast<MDNode>(M->getMetadata()))
       return incorporateMDNode(N);
     if (const auto *MDV = dyn_cast<ValueAsMetadata>(M->getMetadata()))
       return incorporateValue(MDV->getValue());
     return;
   }
 
   if (!isa<Constant>(V) || isa<GlobalValue>(V)) return;

 
 /// incorporateMDNode - This method is used to walk the operands of an MDNode to
 /// find types hiding within.
 void TypeFinder::incorporateMDNode(const MDNode *V) {
   // Already visited?
   if (!VisitedMetadata.insert(V).second)
     return;
 
-  // The arguments in DIArgList are not exposed as operands, so handle such
-  // nodes specifically here.
-  if (const auto *AL = dyn_cast<DIArgList>(V)) {
-    for (auto *Arg : AL->getArgs())
-      incorporateValue(Arg->getValue());
-    return;
-  }
-
   // Look in operands for types.
   for (Metadata *Op : V->operands()) {
     if (!Op)
       continue;
     if (auto *N = dyn_cast<MDNode>(Op)) {
       incorporateMDNode(N);
       continue;
     }
     if (auto *C = dyn_cast<ConstantAsMetadata>(Op)) {
       incorporateValue(C->getValue());
       continue;
     }
   }
 }

llvm/lib/IR/Verifier.cpp

else if (Argument *A = dyn_cast<Argument>(L->getValue()))
     ActualF = A->getParent();
   assert(ActualF && "Unimplemented function local metadata case!");
 
   Assert(ActualF == F, "function-local metadata used in wrong function", L);
 }
 
 void Verifier::visitMetadataAsValue(const MetadataAsValue &MDV, Function *F) {
   Metadata *MD = MDV.getMetadata();
+
+  if (auto *N = dyn_cast<DIArgList>(MD))
+    return;
+
   if (auto *N = dyn_cast<MDNode>(MD)) {
     visitMDNode(*N, AreDebugLocsAllowed::No);
     return;
   }
 
   // Only visit each node once.  Metadata can be mutually recursive, so this
   // avoids infinite recursion here, as well as being an optimization.
   if (!MDNodes.insert(MD).second)
void Verifier::visitDIMacroFile(const DIMacroFile &N) {
   if (auto *Array = N.getRawElements()) {
     AssertDI(isa<MDTuple>(Array), "invalid macro list", &N, Array);
     for (Metadata *Op : N.getElements()->operands()) {
       AssertDI(Op && isa<DIMacroNode>(Op), "invalid macro ref", &N, Op);
     }
   }
 }
 
-void Verifier::visitDIArgList(const DIArgList &N) {
-  AssertDI(!N.getNumOperands(),
-           "DIArgList should have no operands other than a list of "
-           "ValueAsMetadata",
-           &N);
-}
-
 void Verifier::visitDIModule(const DIModule &N) {
   AssertDI(N.getTag() == dwarf::DW_TAG_module, "invalid tag", &N);
   AssertDI(!N.getName().empty(), "anonymous module", &N);
 }
 
 void Verifier::visitDITemplateParameter(const DITemplateParameter &N) {
   AssertDI(isType(N.getRawType()), "invalid type ref", &N, N.getRawType());
 }

llvm/test/Assembler/invalid-diarglist-outside-function.ll

+; RUN: not llvm-as < %s -disable-output 2>&1 | FileCheck %s
+
+; CHECK: <stdin>:[[@LINE+1]]:6: error: expected metadata type
+!0 = !DIArgList()

llvm/test/Assembler/invalid-diexpression-distinct.ll

+; RUN: not llvm-as < %s -disable-output 2>&1 | FileCheck %s
+
+; CHECK: <stdin>:[[@LINE+1]]:6: error: 'distinct' not allowed for !DIExpression
+!0 = distinct !DIExpression()

llvm/test/Bitcode/DIExpression-is-distinct-upgrade.ll

+; RUN: llvm-dis -o - %s.bc | FileCheck %s
+
+!llvm.dbg.cu = !{!1}
+!llvm.module.flags = !{!8, !9}
+
+!0 = distinct !DIGlobalVariable(name: "g", scope: !1, file: !2, line: 1, type: !5, isLocal: false, isDefinition: true)
+!1 = distinct !DICompileUnit(language: DW_LANG_C99, file: !2, producer: "clang", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !3, globals: !4)
+!2 = !DIFile(filename: "a.c", directory: "/")
+!3 = !{}
+!4 = !{!7}
+!5 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+; CHECK: expr: !DIExpression()
+!6 = distinct !DIExpression()
+!7 = !DIGlobalVariableExpression(var: !0, expr: !6)
+!8 = !{i32 2, !"Dwarf Version", i32 4}
+!9 = !{i32 2, !"Debug Info Version", i32 3}

llvm/unittests/IR/MetadataTest.cpp

TEST_F(DIArgListTest, UpdatesOnRAUW) {
   std::unique_ptr<GlobalVariable> GV1(
       new GlobalVariable(Ty, false, GlobalValue::ExternalLinkage));
   auto *MD1 = ValueAsMetadata::get(GV1.get());
   GV0->replaceAllUsesWith(GV1.get());
   EXPECT_EQ(AL->getArgs()[0], CI);
   EXPECT_EQ(AL->getArgs()[1], MD1);
 }
 
+TEST_F(DIArgListTest, UpdatesOnRAUWToExisting) {
+  Module M("M", Context);
+  auto *FTy = FunctionType::get(Type::getVoidTy(Context), false);
+  auto *F = Function::Create(FTy, GlobalValue::ExternalLinkage, "F", &M);
+  auto *BB = BasicBlock::Create(Context, "BB", F);
+
+  auto *Intrinsic =
+      Function::Create(FunctionType::get(Type::getVoidTy(Context),
+                                         Type::getMetadataTy(Context), false),
+                       GlobalValue::ExternalLinkage, "llvm.intrinsic", &M);
+
+  Type *Ty = Type::getInt1PtrTy(Context);
+  ConstantAsMetadata *CI =
+      ConstantAsMetadata::get(ConstantInt::get(Context, APInt(8, 0)));
+  std::unique_ptr<GlobalVariable> GV0(
+      new GlobalVariable(Ty, false, GlobalValue::ExternalLinkage));
+  auto *MD0 = ValueAsMetadata::get(GV0.get());
+
+  SmallVector<ValueAsMetadata *, 2> VMs0;
+  VMs0.push_back(CI);
+  VMs0.push_back(MD0);
+  auto *AL0 = DIArgList::get(Context, VMs0);
+  auto *MV0 = MetadataAsValue::get(Context, AL0);
+  auto *CI0 = CallInst::Create(Intrinsic, MV0, "", BB);
+  AL0 = cast<DIArgList>(MV0->getMetadata());
+  EXPECT_EQ(AL0->getArgs()[0], CI);
+  EXPECT_EQ(AL0->getArgs()[1], MD0);
+
+  std::unique_ptr<GlobalVariable> GV1(
+      new GlobalVariable(Ty, false, GlobalValue::ExternalLinkage));
+  auto *MD1 = ValueAsMetadata::get(GV1.get());
+
+  SmallVector<ValueAsMetadata *, 2> VMs1;
+  VMs1.push_back(CI);
+  VMs1.push_back(MD1);
+  auto *AL1 = DIArgList::get(Context, VMs1);
+  auto *MV1 = MetadataAsValue::get(Context, AL1);
+  auto *CI1 = CallInst::Create(Intrinsic, MV1, "", BB);
+  AL1 = cast<DIArgList>(MV1->getMetadata());
+  EXPECT_EQ(AL1->getArgs()[0], CI);
+  EXPECT_EQ(AL1->getArgs()[1], MD1);
+
+  GV0->replaceAllUsesWith(GV1.get());
+
+  MV0 = cast<MetadataAsValue>(CI0->getArgOperand(0));
+  AL0 = cast<DIArgList>(MV0->getMetadata());
+  MV1 = cast<MetadataAsValue>(CI1->getArgOperand(0));
+  AL1 = cast<DIArgList>(MV1->getMetadata());
+
+  EXPECT_EQ(AL0->getArgs()[0], CI);
+  EXPECT_EQ(AL0->getArgs()[1], MD1);
+  EXPECT_EQ(AL1->getArgs()[0], CI);
+  EXPECT_EQ(AL1->getArgs()[1], MD1);
+  EXPECT_EQ(AL0, AL1);
+}
+
 typedef MetadataTest TrackingMDRefTest;
 
 TEST_F(TrackingMDRefTest, UpdatesOnRAUW) {
   Type *Ty = Type::getInt1PtrTy(Context);
   std::unique_ptr<GlobalVariable> GV0(
       new GlobalVariable(Ty, false, GlobalValue::ExternalLinkage));
   TypedTrackingMDRef<ValueAsMetadata> MD(ValueAsMetadata::get(GV0.get()));
   EXPECT_TRUE(MD->getValue() == GV0.get());