Line-level debug information lost during ARM instruction selection

[orf]

The following Rust code produces five assembly instructions when compiled with optimizations:

pub fn add(left: usize, right: usize) -> usize {
    let result = left + right;
    if result < 10 {
        return 3;
    }
    return result
}

add x9, x1, x0
mov w8, #3
cmp x9, #10
csel x0, x8, x9, lo
ret

However, line-level DWARF information is lost for the if condition, despite it fairly obviously mapping to the “cmp” and “csel” instructions.

compiler explorer links:

• Rust
• llvm IR

It seems that in the “AArch64 Instruction Selection” pass, the conditional is lowered to:

3:gpr64 = SUBSXri %2:gpr64common, 10, 0, implicit-def $nzcv, debug-location !21; example.rs:0
%4:gpr32 = MOVi32imm 3
%5:gpr64 = SUBREG_TO_REG 0, killed %4:gpr32, %subreg.sub_32
%6:gpr64 = CSELXr killed %5:gpr64, %2:gpr64common, 3, implicit $nzcv, debug-location !21; example.rs:0

which as I understand it, is “subtract 10 from the value, and select based on a flag indicating if the result is 0”.

At some point later, this is optimized to a simple comparison, but the line-level information is lost.

[llvmbot]

@llvm/issue-subscribers-debuginfo

Author: Tom Forbes (orf)

The following Rust code produces five assembly instructions when compiled with optimizations:

pub fn add(left: usize, right: usize) -> usize {
    let result = left + right;
    if result < 10 {
        return 3;
    }
    return result
}

add x9, x1, x0
mov w8, #<!-- -->3
cmp x9, #<!-- -->10
csel x0, x8, x9, lo
ret

However, line-level DWARF information is lost for the if condition, despite it fairly obviously mapping to the “cmp” and “csel” instructions.

compiler explorer links:

• Rust
• llvm IR

It seems that in the “AArch64 Instruction Selection” pass, the conditional is lowered to:

3:gpr64 = SUBSXri %2:gpr64common, 10, 0, implicit-def $nzcv, debug-location !21; example.rs:0
%4:gpr32 = MOVi32imm 3
%5:gpr64 = SUBREG_TO_REG 0, killed %4:gpr32, %subreg.sub_32
%6:gpr64 = CSELXr killed %5:gpr64, %2:gpr64common, 3, implicit $nzcv, debug-location !21; example.rs:0

which as I understand it, is “subtract 10 from the value, and select based on a flag indicating if the result is 0”.

At some point later, this is optimized to a simple comparison, but the line-level information is lost.

[llvmbot]

@llvm/issue-subscribers-backend-aarch64

Author: Tom Forbes (orf)

The following Rust code produces five assembly instructions when compiled with optimizations:

pub fn add(left: usize, right: usize) -> usize {
    let result = left + right;
    if result < 10 {
        return 3;
    }
    return result
}

add x9, x1, x0
mov w8, #<!-- -->3
cmp x9, #<!-- -->10
csel x0, x8, x9, lo
ret

However, line-level DWARF information is lost for the if condition, despite it fairly obviously mapping to the “cmp” and “csel” instructions.

compiler explorer links:

• Rust
• llvm IR

It seems that in the “AArch64 Instruction Selection” pass, the conditional is lowered to:

3:gpr64 = SUBSXri %2:gpr64common, 10, 0, implicit-def $nzcv, debug-location !21; example.rs:0
%4:gpr32 = MOVi32imm 3
%5:gpr64 = SUBREG_TO_REG 0, killed %4:gpr32, %subreg.sub_32
%6:gpr64 = CSELXr killed %5:gpr64, %2:gpr64common, 3, implicit $nzcv, debug-location !21; example.rs:0

which as I understand it, is “subtract 10 from the value, and select based on a flag indicating if the result is 0”.

At some point later, this is optimized to a simple comparison, but the line-level information is lost.

[OCHyams]

Thanks for the bug report and reproducer.

Looking at the godbolt reproducer I think the issue is introduced earlier than instruction selection. The select instruction that presumably the CSELXr gets its debug location from has a line 0 debug location.

It looks like it's simplifycfg's hoistSuccIdenticalTerminatorToSwitchOrIf that is performing the transformation that creates this select, which it creates with a line 0 DebugLoc.

Looking at other the similar simplifycfg transformations that insert selects, it seems we do indeed usually apply the DebugLoc from either the cmp or br from the conditional branch to the select. But in hoistSuccIdenticalTerminatorToSwitchOrIf the DebugLoc is taken from the merged-loc of the new unconditional branch.

define noundef range(i64 10, 4) i64 @add(i64 noundef %0, i64 noundef %1) unnamed_addr !dbg !7 {
  %3 = add i64 %0, %1, !dbg !20
  %4 = icmp ult i64 %3, 10, !dbg !22
  br i1 %4, label %6, label %5, !dbg !22

5:                                                ; preds = %2
  br label %7, !dbg !23

6:                                                ; preds = %2
  br label %7, !dbg !24

7:                                                ; preds = %6, %5
  %.0 = phi i64 [ 3, %6 ], [ %3, %5 ], !dbg !21
  ret i64 %.0, !dbg !25
}

Simplfycfg will delete blocks %5 and %6. It creates a new unconditional branch to block %7 in the entry block with the merged DebugLoc of the branches in %5 and %6 (the merged location is a line 0 location). It then inserts a select and applies that merged DebugLoc (propagating the line 0 location to the select). Instead we probably want to grab the DebugLoc from the old to-be-deleted conditional branch in entry.

TL;DR: looks like a bug

cc @Apochens who has been looking into these kinds of issues

[orf]

Thank you for the detailed reply! I would love to have a go at fixing this myself - do you think this would be a suitable thing for a new contributor, or would it be quite tricky to fix? Your description makes it seem quite easy, but you never know 😅

[Apochens]

@OCHyams Thanks! I have inspect the code of simplifycfg's hoistSuccIdenticalTerminatorToSwitchOrIf, in which the two unconditional jumps in if branches are replaced with an new unconditional jump in the entry block and a new select instruction is inserted to produce the result of the old phi. In short, the if-then-else is replaced by a select. [LLVM IR]

IMO, I think this is a reasonable location drop (and the unoptimized icmp could provide the correct debug location of if (result < 10)), as said in the doc:

"Converting an if-then-else CFG diamond into a select. Preserving the debug locations of speculated instructions can make it seem like a condition is true when it’s not (or vice versa), which leads to a confusing single-stepping experience. The rule for dropping locations should apply here."

Though dropping the debug location is recommend in the doc, propagating the merging debugloc (ie., line: 0) would be an alternative.

[orf]

IMO, I think this is a reasonable location drop (and the unoptimized icmp could provide the correct debug location of if (result < 10)), as said in the doc

I do see the rationale for wanting to drop debug locations in this case, but I do just want to point out that the end result is quite confusing: the assembly maps pretty clearly/simply to the input IR/Rust code, and it’s confusing as to why the if conditional debug location isn’t marked.

[OCHyams]

@Apochens said:

"Converting an if-then-else CFG diamond into a select. Preserving the debug locations of speculated instructions can make it seem like a condition is true when it’s not (or vice versa), which leads to a confusing single-stepping experience. The rule for dropping locations should apply here."

Good point - thanks for raising that. I'm wondering about the wording of that rule in the docs.

I had a look at some other cases listed below, taking simplifycfg tests (and adding debug info with debugify in some cases). In these we always take the conditional branch's DebugLoc for the select. I think this does make some sense (and if it's wrong, we have quite a lot of cleaning up to do!). In speculated-store.ll below, we insert a select with the br DebugLoc and we set the speculated store's debug DebugLoc to a merged location (the store stores the selected value) which I think also makes sense but according to the docs you linked should possibly be outright dropped?

@adrian-prantl and @dwblaikie do you think the HowToUpdateDebugInfo is right here, or if it needs updating with more nuance? cc @SLTozer too.

For the reported issue, I think if anything, the source location that makes most sense for this specific transform would be taken from the phi the select is more or less replacing. In this case, that also happens to be a line 0 location (but might not always be).

llvm/test/DebugInfo/simplify-cfg-preserve-dbg-values.ll
https://godbolt.org/z/hYjGdqKoq

llvm/test/DebugInfo/Generic/assignment-tracking/simplifycfg/speculated-store.ll (select gets br line number and speculated store gets line 0 (merged location))
https://godbolt.org/z/Y68GWePoh

llvm/test/Transforms/SimplifyCFG/branch-fold.ll (common_pred)
https://godbolt.org/z/3oG5zdbGv

llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll (test_insertvalue)
https://godbolt.org/z/ns81nxe88

---

As a side note, the icmp -> SUBSXri does also seem to result the DebugLoc going from nonzero to line zero. I haven't had a look as to the cause of this. Usually attributing line 0 is a deliberate choice, but it might be worth investigating why that is too.

---

@orf said:

Do you think this would be a suitable thing for a new contributor?

Absolutely, I think it's the perfect kind of thing to look at as a new contributor - once we've worked out what the behaviour should be. If you're happy to dig around in the instruction selector (which may be less advisable for a new contributor) you are welcome to investigate the icmp -> SUBSXri DebugLoc situation I mentioned above. Don't feel obliged to do this though.

[dwblaikie]

The initial report sounded like "yeah, that's where we drop/merge/line-zero a location" because it described one assembly instruction coming from multiple IR instructions. (could test that in more detail by giving both those IR instructions the exact same DebugLoc, in which case the merged location should preserve that location)

But sounds like there's some more nuance, but reading all the words is a bit hard to follow - is there a clear summary of th issue perhaps using some small IR snippets?

There are certainly cases I remember looking at previously where a select is effectively the merge of several instructions (even just two), which then get lowered to multiple instructions that may map pretty clearly to the original pre-select instructions. But because they got mashed into a single instruction with a single location for a portion of the optimization pipeline, that more detailed correspondence is lost - fixing that is probably non-trivial (being able to carry multiple locations on a single instruction so they can be used if it gets expanded out again). But it doesn't sound like that's the particular issue in this instance (we're only dealing with one output instruction, by the soudns of it).

[OCHyams]

Sure, there's two seperate issues but the discussion is mostly about the select introduced by SimplifyCFG. Here's a summary.

SimplifyCFG squashes an if-then-else-end with empty then and else blocks and a phi in end into the pred, adding a select to calculate the phi value.

At the moment, the select gets the merged location of the brs in then and else.

The core question is, is that right? And if not, what should it be instead?

My position - I'm not certain it's correct because we have documentation that conflicts with many existing cases in LLVM. I think it would be reasonable to instead take the debug location of the conditional branch in the pred (essentially the if) - this appears to be what happens elsewhere in LLVM.

Here is what HowToUpdateDebugInfo says:

When to merge instruction locations

Examples of transformations for which this rule does not apply include:

...

Converting an if-then-else CFG diamond into a select. Preserving the debug locations of speculated instructions can make it seem like a condition is true when it’s not (or vice versa), which leads to a confusing single-stepping experience. The rule for dropping locations should apply here.

First, it actually suggests we drop rather than merge. But IMO the bigger issue is that there are many places in LLVM that don't do this, suggesting either the docs are wrong or there are many places we need to fix up.

In an earlier comment I link to some godbolt examples demonstrating several cases in LLVM where the debug location applied to the select comes from the conditional branch in the pred block (the if).

That seems reasonable to me, as the select is kind of standing in for the now-deleted conditional branch.

In one of the examples (https://godbolt.org/z/Y68GWePoh) we have an if-then-end with a store in if and then that's squashed (store is speculated). In this one the select takes the conditional branch's debug location and the speculated store gets a merged location. This all looks right to me, and I think this is the kind of thing the documentation is getting at, applying a merged location to speculated instructions and not the select itself.