[llvm-commits] [llvm] r57257 - /llvm/trunk/lib/Target/X86/X86InstrInfo.td

[Chris Lattner]

On Oct 8, 2008, at 9:49 AM, Dan Gohman wrote:
>> To put it another way, it looks like (as Dale said) both changes are
>> useful.  The change to make div more closely follow the docs is  
>> useful
>> on its own merits, but the local RA should still be fixed.
>
> Can you explain why?

IMO, the RA should maintain sensible invariants: if an instruction  
defs AL and AH, it should be the same as def'ing AX.

Changing the instruction is goodness just because it more closely  
model's the ISA manual, I don't think it should affect correctness.

> If instruction selectors are working properly, this won't matter.
> So it's making local RA do more work (read: slowing down -O0!),
> and also the MachineInstr DCE used by fast-isel, and as far as I
> can tell the only benefit is that it silently covers up bugs
> elsewhere.
>
> Would the situation be different if we had a MachineInstr-level
> verifier? If it could flag physreg alias abuses, we could be
> more confident that we're not missing any other cases.

A machineinstr verifier is long overdue and I'm sure it would find  
lots of issues!

> This is an area where there are a lot of implicit assumptions, and
> I'd like to understand the assumptions better.

Me too! :)

-Chris