[DebugInfo] Enforce implicit constraints on `distinct` MDNodes
authorScott Linder <Scott.Linder@amd.com>
Mon, 28 Jun 2021 19:40:45 +0000 (19:40 +0000)
committerScott Linder <Scott.Linder@amd.com>
Mon, 28 Jun 2021 21:20:04 +0000 (21:20 +0000)
commit8cd35ad854ab4458fd509447359066ea3578b494
tree661c0db7f9a9eb0d05b03d65803a61cb0c56f030
parentaad87328fabff9382bac0b452c83934515e6d0c8
[DebugInfo] Enforce implicit constraints on `distinct` MDNodes

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.
* Similarly, DIArgList is conceptually always uniqued. It is currently
  restricted to only appearing in contexts where there is no syntax for
  `distinct`, but for consistency it is treated equivalently to
  DIExpression in this patch.
* 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 DIArgList, and
forbids them to be `distinct`. It also implies they 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.

Reviewed By: StephenTozer, t-tye

Differential Revision: https://reviews.llvm.org/D104827
17 files changed:
llvm/docs/LangRef.rst
llvm/docs/SourceLevelDebugging.rst
llvm/include/llvm/AsmParser/LLParser.h
llvm/include/llvm/IR/Metadata.def
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/Bitcode/Reader/MetadataLoader.cpp
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/lib/CodeGen/MIRParser/MIParser.cpp
llvm/lib/IR/AsmWriter.cpp
llvm/lib/IR/DebugInfoMetadata.cpp
llvm/lib/IR/LLVMContextImpl.cpp
llvm/lib/IR/LLVMContextImpl.h
llvm/lib/IR/Metadata.cpp
llvm/test/Assembler/invalid-diarglist-outside-function.ll [new file with mode: 0644]
llvm/test/Assembler/invalid-diexpression-distinct.ll [new file with mode: 0644]
llvm/test/Bitcode/DIExpression-is-distinct-upgrade.ll [new file with mode: 0644]
llvm/test/Bitcode/DIExpression-is-distinct-upgrade.ll.bc [new file with mode: 0644]