[Lldb-commits] [PATCH] D91734: [FastISel] Flush local value map on every instruction

Tue Dec 1 14:55:05 PST 2020

rnk added a comment.

I see. We should give that constant materialization a location. It looks like it is coming from a phi node. The IR looks like this:

    %6 = icmp ne i32 %5, 0, !dbg !11
    br i1 %6, label %7, label %10, !dbg !12

  7:                                                ; preds = %2
    %8 = load i32, i32* %4, align 4, !dbg !13
    %9 = icmp ne i32 %8, 0, !dbg !13
    br label %10

  10:                                               ; preds = %7, %2
    %11 = phi i1 [ false, %2 ], [ %9, %7 ], !dbg !14   ;;;; Probably where the zero comes from
    %12 = zext i1 %11 to i32, !dbg !11
    ret i32 %12, !dbg !15

The PHI node has location !14, which is a line 0 location. Is there a reason we give this PHI a line 0 location, when it's built by the frontend for the conditional operator? IMO we should use the location of the `br` instruction, which will be the location of the conditional operator (`&&` at the source level).

Paul has already shown that flushing the local value map improves debug info quality in general. If we can't fix all the gdb test suite failures, IMO we should consider XFAILing them and moving on.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91734