[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?
More information about the llvm-commits