[PATCH] D158324: Revert "[RegisterCoalescing] Don't move COPY if it would interfere with another value"

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added a comment.

In D158324#4600616 <https://reviews.llvm.org/D158324#4600616>, @arsenm wrote:

> In D158324#4600553 <https://reviews.llvm.org/D158324#4600553>, @bjope wrote:
>
>> If so it would be nice to say something in the commit message about that (that the code is reverted even if there is no proof that it was incorrect?).
>
> Not sure what you mean, if you add -verify-coalescing to the test it fails.

The test case failed verification even before (or without) running the register-coalescer. So as the commit message says, the test case seems broken.

But I thought it was a bit unclear in the commit message that it also removes a defensive check in removePartialRedundancy. I doubt that we will trigger the optimization in more cases (and if it does, then that would be faulty). So something saying that we no longer think that the problematic situation can occur for valid input. And if that assumption is wrong, then we hopefully will catch it in regression tests with verifiers activates since the defensive check in removePartialRedundancy is removed.

I've however found our downstream test case now. And that one is not failing verification on the input.

I came up with these adjustments, that without the patch by uabelho would end up with "Assertion `SlotIndex::isEarlierInstr(Def, S->start) && "Already live at def"' failed.":

  # RUN: llc -march=mips64 -o - %s -run-pass=register-coalescer -verify-coalescing | FileCheck %s
  
  ---
  name:            f
  tracksRegLiveness: true
  body:             |
    bb.0:
      %0:gpr32 = ADDiu $zero, 0
      %1:gpr32 = COPY %0
      %1:gpr32 = ADDiu %1, 1
      BEQ %0, $zero, %bb.3, implicit-def $at
      J %bb.1, implicit-def dead $at
  
    bb.1:
      J %bb.2, implicit %1, implicit-def dead $at
  
    bb.2:
      %1:gpr32 = COPY %0
      %0:gpr32 = COPY %1
      BEQ %0:gpr32, $zero, %bb.2, implicit-def $at
  
    bb.3:
      %4:gpr32 = ADDiu %1, 1
  
  ...

Afaict that test case passes verification on the test input.


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

https://reviews.llvm.org/D158324