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

[Matthias Braun via llvm-commits]

MatzeB added a comment.

In http://reviews.llvm.org/D20456#439675, @mbodart wrote:

> I did some more digging re the tied operand check, and I think the idea there is that a tied operand will still be live into the instruction, so apparently the author thought this meant the def need not be treated as a live range killer.  Mucking with that assumption is not something I want to delve into with this particular change.


My understanding is that some things cannot be renamed legally and KeepRegs collects those:
For calls the ABI forces some values to some register, and for two-address code (the tied-operand case) we cannot simply rename the input because it has to match the output. That much of the code seems fine to me.

> Matthias, this isn't just being enabled now.  The X86 post scheduler had been enabled for atom, bonnell, btver2 and slm.

>  I agree with your general concern re the stability of the antidepence breaker, simply from paranoia after staring at the code.

>  But a complete overhaul is a longer term project.

> 

> For the short term I agree with Quentin that a small, safe, patch is reasonable.

>  But per one of Sanjay's bugzilla comments, it may also be reasonable to disable the antidepence breaker for X86.

>  We'd have to run some performance tests to verify that.


Yes having a fix for the issue at hand is perfectly fine, I am not trying to talk you into rewriting the pass. I was only objecting to the patch as it stands right now because adding the super/sub register iterator this way for a tied operand seems to fix the issue by accident not by design.


http://reviews.llvm.org/D20456