[PATCH] D88175: [DebugInfo] Add new metadata, DIArgList, for referencing a list of SSA values inside a debug variable intrinsic

Duncan P. N. Exon Smith via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 6 12:12:31 PST 2021


As a drive-by comment, I'll add that `MDTuple` was originally meant to be a `final` version of `MDNode`, as in if `isa<MDTuple>` returns `true` then you know you have a standard metadata tuple, and not some other special type. Inheriting from `MDNode` sounds more in line with the original intent to me. (I'm not sure it's important to maintain that as an invariant, but thought I'd mention it.)

> On 2021 Jan  6, at 10:08, Stephen Tozer via Phabricator <reviews at reviews.llvm.org> wrote:
> 
> StephenTozer added a comment.
> 
> In D88175#2450058 <https://reviews.llvm.org/D88175#2450058>, @aprantl wrote:
> 
>> Although — no that I read the reply — it looks like the MDTuple question hasn't actually been answered ;-)
> 
> Apologies for taking a while to get back around to answering it, I've had to do some step retracing. The short answer is that MDTuple doesn't actually contain any useful common code for DIArgList. The longer answer is that MDTuple comes with built-in functionality for an arbitrary tuple of metadata, which is similar to what we want. However, what it isn't innately equipped to handle is LocalAsMetadata, which is unique among the many types of metadata in LLVM as being the only kind that needs a function context to be parsed. Therefore, all the rules around parsing and printing LocalAsMetadata entries are special-case; the same is also true of RAUW, which works by default for normal Values but uses a separate map and code path to handle ValueAsMetadata and MetadataAsValue.
> 
> In terms of parsing, processing, and printing, DIArgList is much more similar to a LocalAsMetadata than it is to an MDTuple, and most of the "new" code for handling DIArgList is side-by-side with the LocalAsMetadata code, and heavily based off of it. Because of this, even though we could technically base DIArgList off of MDTuple, we wouldn't be able to use most of the existing code; at best, we would be moving all of the code unique for DIArgLists into the `parseMDTuple` function. If anything, DIArgList might not actually need to be an MDNode at all, but just plain Metadata - I started with an implementation similar to DIExpression, which is based on MDNode, but I haven't confirmed that it's necessary.
> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D88175/new/
> 
> https://reviews.llvm.org/D88175
> 



More information about the llvm-commits mailing list