[PATCH] D20456: [CodeGen] Fix problem with X86 byte registers in CriticalAntiDepBreaker

[Matthias Braun via llvm-commits]

MatzeB requested changes to this revision.
MatzeB added a comment.
This revision now requires changes to proceed.

Looking into the testcase this seems to be a problem with tracking which registers are actually free and usable, the algorithm somehow fails to recognize that %ch is not free when %ecx is used.

The KeepRegsSet on the other hand excludes some registers from renaming because that would change program semantics. This does not appear to be the right fix, the fact that the `andb` in the testcase is a tied operand seems accidental to me. For a proper fix we should have findSuitableFreeRegister() report false for %ch.

Some possible ways to proceed:

- It would be good to have a minimal test case as a .mir file which just does "llc -run-pass post-RA-scheduler -verify-machineinstrs xxx.mir" that serves well as a testcase and should also make it realistic to single step through the algorithm with a debugger...
- I wonder if isNewRegClobberedByRefs() should check subregisters instead of just `if(... CheckOper.getReg() != NewReg) continue;`
- The `We just went back in time and modified history; ...` part also does not seem to respect subregisters/superregisters.


http://reviews.llvm.org/D20456