[PATCH] D75555: [GlobalISel][Localizer] Enable intra-block localization of already-local uses.

[Amara Emerson via Phabricator via llvm-commits]

aemerson added a comment.

In D75555#1906391 <https://reviews.llvm.org/D75555#1906391>, @omjavaid wrote:

> Hi @aemerson this change caused a regression in lldb testsuite on AArch64/Linux. Apparently stepping gets affected by your change. Kindly revisit.
>
> I have temporarily reverted this change.
>
> http://lab.llvm.org:8011/builders/lldb-aarch64-ubuntu/builds/2182


That test looks...wrong. single_step_only_steps_one_instruction() looks to be checking that exactly *one* instruction is needed to step between each line of the test code:

  static void swap_chars() {
    g_c1 = '1';
    g_c2 = '0';
  
    g_c1 = '0';
    g_c2 = '1';
  }

Before it was just pure luck that the 4 stores were scheduled consecutively:

  __ZL10swap_charsv:
    mov w8, #0x31
    adrp  x9, 9 ; 0x100010000
    add x9, x9, #0x240 
    mov w10, #0x30 
    adrp  x11, 9 ; 0x100010000 
    add x11, x11, #0x241 
    strb  w8, [x9] 
    strb  w10, [x11] 
    strb  w10, [x9] 
    strb  w8, [x11] 
    ret

with this patch, we emit some address computation after the first store:

  __ZL10swap_charsv:
    adrp  x8, 9 ; 0x100010000
    add x8, x8, #0x240
    mov w9, #0x31
    strb  w9, [x8]
    adrp  x10, 9 ; 0x100010000
    add x10, x10, #0x241
    mov w11, #0x30
    strb  w11, [x10]
    strb  w11, [x8]
    strb  w9, [x10]
    ret

which is perfectly valid. I'm not sure how to fix this test because even if we change the assertion to be something like assert(step_count <= max_steps), it's still fragile.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75555