[llvm-dev] Can DBG_VALUE instructions use undefined vregs?

[Amara Emerson via llvm-dev]

One GlobalISel compile time optimization patch (https://reviews.llvm.org/D109750) has generated some debate over whether it’s semantically allowed/not-an-error for a DBG_VALUE machine instruction to have a use of a vreg that doesn’t have a corresponding definition.

We talked internally with Adrian and Vedant and didn’t come to a strong conclusion either way. Does anyone have thoughts here?

Thanks,

Amara

[Matthias Braun via llvm-dev]

I can't really be speak about debug instructions in particular. For machine instructions in general you just mark the use operand with "undef" and you are good...

- Matthias

> _______________________________________________
> LLVM Developers mailing list
> llvm...@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

_______________________________________________
LLVM Developers mailing list
llvm...@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

[Amara Emerson via llvm-dev]

That’s what we were doing in the legalizer, but calling eraseFromParentAndMarkDBGValuesForRemoval() is much more expensive than a plain eraseFromParent(), so the motivating change was to use a heuristic to avoid calling it when it was very likely that it wasn’t going to have a debug user.

Whichever answer we get is fine, we can have a cleanup phase after legalization over all vregs in MachineRegisterInfo to mark ones without defs as undef, but I’d like to avoid doing that if we can so we don’t spend the extra compile time.

Amara

[Jeremy Morse via llvm-dev]

Hi,

On Tue, Oct 5, 2021 at 11:29 PM Amara Emerson via llvm-dev
<llvm...@lists.llvm.org> wrote:
> One GlobalISel compile time optimization patch (https://reviews.llvm.org/D109750) has generated some debate over whether it’s semantically allowed/not-an-error for a DBG_VALUE machine instruction to have a use of a vreg that doesn’t have a corresponding definition.

I don't think there's a semantic reason why DBG_VALUEs of undefined
vregs should be disallowed -- they can be interpreted the same as a
DBG_VALUE $noreg, just in a non-canonical form. They would correctly
become DBG_VALUE $noreg's when LiveDebugVariables runs, as the vreg
wouldn't be live.

In practical terms, it could slightly mislead people reading MIR, and
there might be code out there that expects to find a vreg definition
for anything that DBG_VALUEs refer to, which I imagine would be fairly
easy to work around.

--
Thanks,
Jeremy

[Matthias Braun via llvm-dev]

So you are asking about whether

... no definition of %X anywhere ...
DBG_VALUE %x ; No undef flag here

is fine. I think technically nothing will stop you. DBG_VALUE instructions are not real uses and ignored for anything semantically interesting like liveness etc. Thinking about it from this angle it wouldn't be that far fetched to just mark all DBG_VALUE operands as `undef`.

On the other hand motivating this with `eraseFromParentAndMarkDBGValuesForRemoval` vs. `eraseFromParent` feels surprising... Somehow I would have assumed that once you finished all the work of creating a DBG_VALUE instruction and a register and multiple passes reading those instructions and operands, that one more iteration over the operands wouldn't really make a noticeable performance difference...

- Matthias

> On Oct 5, 2021, at 4:24 PM, Amara Emerson <am...@apple.com> wrote:
>
> That’s what we were doing in the legalizer, but calling eraseFromParentAndMarkDBGValuesForRemoval() is much more expensive than a plain eraseFromParent(), so the motivating change was to use a heuristic to avoid calling it when it was very likely that it wasn’t going to have a debug user.
>
> Whichever answer we get is fine, we can have a cleanup phase after legalization over all vregs in MachineRegisterInfo to mark ones without defs as undef, but I’d like to avoid doing that if we can so we don’t spend the extra compile time.
>
> Amara
>
>> On Oct 5, 2021, at 3:37 PM, Matthias Braun <matt...@fb.com> wrote:
>>
>> I can't really be speak about debug instructions in particular. For machine instructions in general you just mark the use operand with "undef" and you are good...
>>
>> - Matthias
>>
>>> On Oct 5, 2021, at 3:29 PM, Amara Emerson via llvm-dev <llvm...@lists.llvm.org> wrote:
>>>

>>> One GlobalISel compile time optimization patch (https://reviews.llvm.org/D109750 ) has generated some debate over whether it’s semantically allowed/not-an-error for a DBG_VALUE machine instruction to have a use of a vreg that doesn’t have a corresponding definition.

[Amara Emerson via llvm-dev]

> On Oct 6, 2021, at 9:09 AM, Matthias Braun <matt...@fb.com> wrote:
>
> So you are asking about whether
>
> ... no definition of %X anywhere ...
> DBG_VALUE %x ; No undef flag here
>
> is fine. I think technically nothing will stop you. DBG_VALUE instructions are not real uses and ignored for anything semantically interesting like liveness etc. Thinking about it from this angle it wouldn't be that far fetched to just mark all DBG_VALUE operands as `undef`.
>

> On the other hand motivating this with `eraseFromParentAndMarkDBGValuesForRemoval` vs. `eraseFromParent` feels surprising... Somehow I would have assumed that once you finished all the work of creating a DBG_VALUE instruction and a register and multiple passes reading those instructions and operands, that one more iteration over the operands wouldn't really make a noticeable performance difference…
The move to use eraseFromParent() was in a part of the legalizer that can see very high numbers of artifact instructions being created and deleted as they’re combined away, so this region of code can be hot on a profile.