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

[David Blaikie via Phabricator via lldb-commits]

dblaikie added a comment.

In D91734#2432188 <https://reviews.llvm.org/D91734#2432188>, @probinson wrote:

> See D92606 <https://reviews.llvm.org/D92606> for a front-end patch to improve locations in the IR.
> That, plus reapplying this patch, should help out GDB.  I haven't had a chance to run the suite myself with both patches applied and I'm basically off tomorrow, so if @dblaikie doesn't have a chance, I'll do that on Monday.

Had a go - certainly creates a better debugging experience, but still fails the gdb test in question.

The code in the original test looks something like:

  int multi_line_while_conditional (int a, int b, int c)
  {
    while (a /* set breakpoint 4 here */
        && b 
        && c) 
      { 
        a--, b--, c--;
      }
    return 0;
  }

And with these changes together, breaking on the function breaks on line 5 (presumably because this is creating the constant used by the second '&&' which is on line 5) instead of line 3. Not the worst thing, but I imagine given Sony's interest in less "jumpy" line tables, this might not be super desirable.

Yeah, the gdb experience is less than ideal:

  13        multi_line_while_conditional(1, 1, 1);
  (gdb) s
  multi_line_while_conditional (a=1, b=1, c=1) at test.cpp:5
  5             && c) 
  (gdb) n
  3         while (a /* set breakpoint 4 here */
  (gdb) 
  4             && b 
  (gdb) 
  5             && c) 
  (gdb) 
  4             && b 
  (gdb) 
  5             && c) 
  (gdb) 
  3         while (a /* set breakpoint 4 here */
  (gdb) 
  7             a--, b--, c--;

Compared with (without any of these patches):

  13        multi_line_while_conditional(1, 1, 1);
  (gdb) s
  multi_line_while_conditional (a=1, b=1, c=1) at test.cpp:3
  3         while (a /* set breakpoint 4 here */
  (gdb) n
  4             && b 
  (gdb) 
  5             && c) 
  (gdb) 
  3         while (a /* set breakpoint 4 here */
  (gdb) 
  7             a--, b--, c--;

And because I was curious about /exactly/ which tokens the debugger was stepping to, I split out the tokens onto their own lines:
With patch:

  18        multi_line_while_conditional(1, 1, 1);
  (gdb) s
  multi_line_while_conditional (a=1, b=1, c=1) at test.cpp:8
  8             && 
  (gdb) n
  5               a 
  (gdb) 
  6             && 
  (gdb) 
  8             && 
  (gdb) 
  7             b 
  (gdb) 
  8             && 
  (gdb) 
  9             c
  (gdb) 
  3         while 
  (gdb) 
  12            a--, b--, c--;

Without patch/with trunk:

  18        multi_line_while_conditional(1, 1, 1);
  (gdb) s
  multi_line_while_conditional (a=1, b=1, c=1) at test.cpp:5
  5               a 
  (gdb) n
  6             && 
  (gdb) 
  7             b 
  (gdb) 
  8             && 
  (gdb) 
  9             c
  (gdb) 
  3         while 
  (gdb) 
  12            a--, b--, c--;

Maybe it's OK? But at least it's 'interesting' enough that might deserve some extra scrutiny.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91734