[PATCH] D22083: Revert "[X86]: Improve Liveness checking for X86FixupBWInsts.cpp"

[Matthias Braun via llvm-commits]

MatzeB added a comment.

In http://reviews.llvm.org/D22083#476412, @kbsmith1 wrote:

> What have you done to prove to yourself that this code is no longer necessary?
>  If unnecessary, then both before and after the change, the code for 401.bzip2 from spec20006 ought to
>  be identical.


I can certainly explain to you what caused these ridiculous live-in lists. Basically looking at a set of register units and then adding all registers with that unit leads to al, ax, eax, rax getting added even if just al is live (additonally the code would unnecessarily add all the pristine registers). So I am pretty sure these wrong live-in lists causes the way too conversative lifeness when walking the block from the end.

> One way of showing that this code is no longer necessary would be to use the new code and simply

>  add an assertion after line 389 that NewMI was always nullptr.  Once that code had been in for a while,

>  you could make the extra forward pass be a debug-only code in an attempt to catch other regressions

>  that might occur in live-reg tracking.

> 

> Also, I kind of liked the better abstraction of tryReplaceInstr. I thought that was an improvement, and is another

>  reason I don't think just a simple revert is the ideal fix.


Fair enough do a round of testing with the full llvm test-suite including externals (spec95,spec2k,spec2k6) to see that there is really no change. Keeping the tryReplaceInstr() function around is fine, this review is mainly here to get the discussion started :)


Repository:
  rL LLVM

http://reviews.llvm.org/D22083