[PATCH] D101373: [DebugInfo] Drop DBG_VALUE_LISTs with an excessive number of debug operands

Tue Apr 27 17:33:34 PDT 2021

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

In D101373#2721021 <https://reviews.llvm.org/D101373#2721021>, @StephenTozer wrote:

> In D101373#2720962 <https://reviews.llvm.org/D101373#2720962>, @dblaikie wrote:
>
>> Oh, right. I just realized this test doesn't have to be especially realistic because it's only about the length of the expression - not how we got such a long expression in the first place?
>>
>> So perhaps we could include a comment describing the real-ish situation, but then make the test case really simple/obvious? - it doesn't need any IR instructions really, and can have an expression that's simple/uninteresting in some way? Like adding together the constant 1 repeatedly?
>
> Yes, that would work - I've checked that as well, and there can be an LL test case of just over 30 lines (although to be more specific, they have to be different constants - identical values will be coalesced just above the assert in question). One thing is that I suspect we might have change it back to the register form in the future however; on the topic of reducing the size of expressions, DIExpression optimization has come up before. The simplest forms of this to implement would be removal of no-ops subexpressions (like "multiply by 1), and simplifying constant subexpressions. This would cause an issue with the test later on, at which point we'd revert back to the test currently in this patch.

Guess you could change it to an MIR test case now that it can be further reduced like this - but up to you, I wouldn't insist on it.

If there's some way to make an irreducable expression, that'd be handy, but nothing comes immediately to mind... perhaps a parameter that's a pointer to an array and an expression that adds many elements from that array together? Not sure. If there's something reasonably easy to do there it would be nice if the test was more reliable - but I wouldn't spend ages on it.

> That being said, we don't have to bend over backwards to accomodate future potential changes; I can just add a note to the test pointing to this review to ensure that if it breaks later as the result of such a change being added, whoever picks it up doesn't have to go back to square 1 on finding a reproducer.

Yeah, some comments/breadcrumbs should suffice.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101373/new/

https://reviews.llvm.org/D101373