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

[Paul Robinson via Phabricator via lldb-commits]

probinson added a comment.

@rnk is correct that the phi's source location is propagating to the xorl.  I modified the .ll file to put a different line number on the phi, and the xorl picked it up.  This is clearly on the front-end to get right.

If we want to get down to cases, there are other poor choices of source location in this tiny example.  The statement source is

  return a && b;

The 'a' is at column 10, the '&&' at 12, the 'b' at 15.
Now consider the load/test of 'a':

    %0 = load i32, i32* %a.addr, align 4, !dbg !19
    %tobool = icmp ne i32 %0, 0, !dbg !19
    br i1 %tobool, label %land.rhs, label %land.end, !dbg !20
  ...
  !19 = !DILocation(line: 3, column: 10, scope: !12)
  !20 = !DILocation(line: 3, column: 12, scope: !12)

The load and icmp both use !19, the source location of 'a' in the expression; the br uses !20, the '&&'.  All seems good. Now consider the 'b' part:

    %1 = load i32, i32* %b.addr, align 4, !dbg !21
    %tobool1 = icmp ne i32 %1, 0, !dbg !20
    br label %land.end
  ...
  !20 = !DILocation(line: 3, column: 12, scope: !12)
  !21 = !DILocation(line: 3, column: 15, scope: !12)

The load uses !21, 'b', but the icmp points to the '&&', and the br points nowhere.  This is at least inconsistent with how the LHS of the expression is handled, and I would argue it's actually wrong.

This gets reflected in the generated code:

  	.loc	1 0 0 prologue_end              # gdb-fail.c:0:0
  	xorl	%eax, %eax                      ;;;; because of the phi's line-0
                                          # kill: def $al killed $al killed $eax
  	.loc	1 3 10                          # gdb-fail.c:3:10
  	cmpl	$0, -4(%rbp)                    ;;;; load/compare of 'a' at column 10
  	movb	%al, -9(%rbp)                   # 1-byte Spill
  	.loc	1 3 12 is_stmt 0                # gdb-fail.c:3:12
  	je	.LBB1_2                         ;;;; the je attributed to the &&
  # %bb.1:                                # %land.rhs
  	cmpl	$0, -8(%rbp)                    ;;;; load/compare of 'b' at column 12 (??) not 15
  	setne	%al
  	movb	%al, -9(%rbp)                   # 1-byte Spill
  .LBB1_2:                                # %land.end
  	.loc	1 0 12                          # gdb-fail.c:0:12
  	movb	-9(%rbp), %al                   # 1-byte Reload

I'd also be happier if the reload was attributed to the same source location as the following instructions, but that's probably a whole 'nother can of worms.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91734