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

Matthias Braun via llvm-commits llvm-commits at lists.llvm.org
Wed May 25 11:38:39 PDT 2016


MatzeB added a comment.

Please do not push the fix in the current form.

In http://reviews.llvm.org/D20456#439547, @qcolombet wrote:

> I am going to state the obvious but, given we have introduced regressions I am fine with a hacky fix as long as it is low risk. I do not have a good sense of risk at the moment, does anyone have a good feeling on that?
>  For the longer term solution, it sounds like we should really step back and take the time to clarify how the code is working and go for a proper redesign.


Please do not push the fix in the current form. The fact that there was a tied operand involved seems accidental to me it very likely does not fix the real problem. I believe the only reason we are not seeing the problem more often is that subregister usage is so extremely rare on x86 (I think I only found a handful places in the whole of the llvm test-suite last time I looked).

> I don’t think we have a code owner for that pass, so if we should to go that direction, someone has to step up for the long term fix.


The current code *DepBreaker.* code is bad, does liveness in untypical ways that makes it very hard to understand and I am pretty sure it was always broken. But maybe we just shouldn't enable it then before it is fixed?


http://reviews.llvm.org/D20456





More information about the llvm-commits mailing list